2017-09-12 09:22:33

by Arend van Spriel

[permalink] [raw]
Subject: [PATCH 0/7] brcmfmac: scan code cleanup and more

This series has following changes:
* cleanup scanning code.
* deal with FWHALT indication.
* fix for promiscuous mode.

The patches apply to master branch of wireless-drivers-next
repository.

Arend van Spriel (6):
brcmfmac: handle FWHALT mailbox indication
brcmfmac: cleanup brcmf_cfg80211_escan() function
brcmfmac: use msecs_to_jiffies() instead of calculation using HZ
brcmfmac: get rid of brcmf_cfg80211_escan() function
brcmfmac: get rid of struct brcmf_cfg80211_info::active_scan field
brcmfmac: move configuration of probe request IEs

Franky Lin (1):
brcmfmac: disable packet filtering in promiscuous mode

.../broadcom/brcm80211/brcmfmac/cfg80211.c | 162 ++++-----------------
.../broadcom/brcm80211/brcmfmac/cfg80211.h | 2 -
.../wireless/broadcom/brcm80211/brcmfmac/core.c | 38 +++++
.../wireless/broadcom/brcm80211/brcmfmac/core.h | 1 +
.../net/wireless/broadcom/brcm80211/brcmfmac/p2p.c | 11 +-
.../wireless/broadcom/brcm80211/brcmfmac/sdio.c | 14 +-
6 files changed, 77 insertions(+), 151 deletions(-)

--
1.9.1


2017-09-12 09:22:33

by Arend van Spriel

[permalink] [raw]
Subject: [PATCH 5/7] brcmfmac: get rid of brcmf_cfg80211_escan() function

The function brcmf_cfg80211_escan() is only called by brcmf_cfg80211_scan()
so there is no reason to split in two function especially since the latter
does not do an awful lot.

Reviewed-by: Hante Meuleman <[email protected]>
Reviewed-by: Pieter-Paul Giesberts <[email protected]>
Reviewed-by: Franky Lin <[email protected]>
Signed-off-by: Arend van Spriel <[email protected]>
---
.../broadcom/brcm80211/brcmfmac/cfg80211.c | 34 +++++++---------------
1 file changed, 10 insertions(+), 24 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index 8e7717a..34c2945 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -1082,13 +1082,16 @@ static void brcmf_escan_prep(struct brcmf_cfg80211_info *cfg,
}

static s32
-brcmf_cfg80211_escan(struct wiphy *wiphy, struct brcmf_cfg80211_vif *vif,
- struct cfg80211_scan_request *request)
+brcmf_cfg80211_scan(struct wiphy *wiphy, struct cfg80211_scan_request *request)
{
struct brcmf_cfg80211_info *cfg = wiphy_to_cfg(wiphy);
- s32 err;
+ struct brcmf_cfg80211_vif *vif;
+ s32 err = 0;

- brcmf_dbg(SCAN, "START ESCAN\n");
+ brcmf_dbg(TRACE, "Enter\n");
+ vif = container_of(request->wdev, struct brcmf_cfg80211_vif, wdev);
+ if (!check_vif_up(vif))
+ return -EIO;

if (test_bit(BRCMF_SCAN_STATUS_BUSY, &cfg->scan_status)) {
brcmf_err("Scanning already: status (%lu)\n", cfg->scan_status);
@@ -1113,6 +1116,8 @@ static void brcmf_escan_prep(struct brcmf_cfg80211_info *cfg,
if (vif == cfg->p2p.bss_idx[P2PAPI_BSSCFG_DEVICE].vif)
vif = cfg->p2p.bss_idx[P2PAPI_BSSCFG_PRIMARY].vif;

+ brcmf_dbg(SCAN, "START ESCAN\n");
+
cfg->scan_request = request;
set_bit(BRCMF_SCAN_STATUS_BUSY, &cfg->scan_status);

@@ -1132,31 +1137,12 @@ static void brcmf_escan_prep(struct brcmf_cfg80211_info *cfg,
return 0;

scan_out:
+ brcmf_err("scan error (%d)\n", err);
clear_bit(BRCMF_SCAN_STATUS_BUSY, &cfg->scan_status);
cfg->scan_request = NULL;
return err;
}

-static s32
-brcmf_cfg80211_scan(struct wiphy *wiphy, struct cfg80211_scan_request *request)
-{
- struct brcmf_cfg80211_vif *vif;
- s32 err = 0;
-
- brcmf_dbg(TRACE, "Enter\n");
- vif = container_of(request->wdev, struct brcmf_cfg80211_vif, wdev);
- if (!check_vif_up(vif))
- return -EIO;
-
- err = brcmf_cfg80211_escan(wiphy, vif, request);
-
- if (err)
- brcmf_err("scan error (%d)\n", err);
-
- brcmf_dbg(TRACE, "Exit\n");
- return err;
-}
-
static s32 brcmf_set_rts(struct net_device *ndev, u32 rts_threshold)
{
s32 err = 0;
--
1.9.1

2017-09-12 09:22:33

by Arend van Spriel

[permalink] [raw]
Subject: [PATCH 1/7] brcmfmac: handle FWHALT mailbox indication

The firmware uses a mailbox to communicate to the host what is going
on. In the driver we validate the bit received. Various people seen
the following message:

brcmfmac: brcmf_sdio_hostmail: Unknown mailbox data content: 0x40012

Bit 4 is cause of this message, but this actually indicates the firmware
has halted. Handle this bit by giving a more meaningful error message.

Reviewed-by: Hante Meuleman <[email protected]>
Reviewed-by: Pieter-Paul Giesberts <[email protected]>
Reviewed-by: Franky Lin <[email protected]>
Signed-off-by: Arend van Spriel <[email protected]>
---
drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
index 613caca..3d79664 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
@@ -260,10 +260,11 @@ struct rte_console {
#define I_HMB_HOST_INT I_HMB_SW3 /* Miscellaneous Interrupt */

/* tohostmailboxdata */
-#define HMB_DATA_NAKHANDLED 1 /* retransmit NAK'd frame */
-#define HMB_DATA_DEVREADY 2 /* talk to host after enable */
-#define HMB_DATA_FC 4 /* per prio flowcontrol update flag */
-#define HMB_DATA_FWREADY 8 /* fw ready for protocol activity */
+#define HMB_DATA_NAKHANDLED 0x0001 /* retransmit NAK'd frame */
+#define HMB_DATA_DEVREADY 0x0002 /* talk to host after enable */
+#define HMB_DATA_FC 0x0004 /* per prio flowcontrol update flag */
+#define HMB_DATA_FWREADY 0x0008 /* fw ready for protocol activity */
+#define HMB_DATA_FWHALT 0x0010 /* firmware halted */

#define HMB_DATA_FCDATA_MASK 0xff000000
#define HMB_DATA_FCDATA_SHIFT 24
@@ -1094,6 +1095,10 @@ static u32 brcmf_sdio_hostmail(struct brcmf_sdio *bus)
offsetof(struct sdpcmd_regs, tosbmailbox));
bus->sdcnt.f1regdata += 2;

+ /* dongle indicates the firmware has halted/crashed */
+ if (hmb_data & HMB_DATA_FWHALT)
+ brcmf_err("mailbox indicates firmware halted\n");
+
/* Dongle recomposed rx frames, accept them again */
if (hmb_data & HMB_DATA_NAKHANDLED) {
brcmf_dbg(SDIO, "Dongle reports NAK handled, expect rtx of %d\n",
@@ -1151,6 +1156,7 @@ static u32 brcmf_sdio_hostmail(struct brcmf_sdio *bus)
HMB_DATA_NAKHANDLED |
HMB_DATA_FC |
HMB_DATA_FWREADY |
+ HMB_DATA_FWHALT |
HMB_DATA_FCDATA_MASK | HMB_DATA_VERSION_MASK))
brcmf_err("Unknown mailbox data content: 0x%02x\n",
hmb_data);
--
1.9.1

2017-09-26 12:05:43

by James Hughes

[permalink] [raw]
Subject: Re: [1/7] brcmfmac: handle FWHALT mailbox indication

On 26 September 2017 at 12:11, Arend van Spriel
<[email protected]> wrote:
> On 25-09-17 11:30, James Hughes wrote:
>>
>> On 25 September 2017 at 10:26, James Hughes
>> <[email protected]> wrote:
>>>
>>> On 25 September 2017 at 09:18, Kalle Valo <[email protected]> wrote:
>>>>
>>>> Arend Van Spriel <[email protected]> wrote:
>>>>
>>>>> The firmware uses a mailbox to communicate to the host what is going
>>>>> on. In the driver we validate the bit received. Various people seen
>>>>> the following message:
>>>>>
>>>>> brcmfmac: brcmf_sdio_hostmail: Unknown mailbox data content: 0x4001=
2
>>>>>
>>>>> Bit 4 is cause of this message, but this actually indicates the
>>>>> firmware
>>>>> has halted. Handle this bit by giving a more meaningful error message=
.
>>>>>
>>>>> Reviewed-by: Hante Meuleman <[email protected]>
>>>>> Reviewed-by: Pieter-Paul Giesberts <[email protected]=
m>
>>>>> Reviewed-by: Franky Lin <[email protected]>
>>>>> Signed-off-by: Arend van Spriel <[email protected]>
>>>>
>>>>
>>>> Failed to compile:
>>>>
>>>> drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c: In function
>>>> =E2=80=98brcmf_p2p_escan=E2=80=99:
>>>> drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c:695:23: error:
>>>> =E2=80=98BRCMF_SCANTYPE_ACTIVE=E2=80=99 undeclared (first use in this =
function)
>>>> sparams->scan_type =3D BRCMF_SCANTYPE_ACTIVE;
>>>> ^
>>>> drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c:695:23: note:
>>>> each undeclared identifier is reported only once for each function it
>>>> appears in
>>>> make[6]: *** [drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.o]
>>>> Error 1
>>>> make[6]: *** Waiting for unfinished jobs....
>>>> make[5]: *** [drivers/net/wireless/broadcom/brcm80211/brcmfmac] Error =
2
>>>> make[4]: *** [drivers/net/wireless/broadcom/brcm80211] Error 2
>>>> make[3]: *** [drivers/net/wireless/broadcom] Error 2
>>>> make[2]: *** [drivers/net/wireless] Error 2
>>>> make[1]: *** [drivers/net] Error 2
>>>> make: *** [drivers] Error 2
>>>>
>>>> 7 patches set to Changes Requested.
>>>>
>>>> 9948825 [1/7] brcmfmac: handle FWHALT mailbox indication
>>>> 9948831 [2/7] brcmfmac: disable packet filtering in promiscuous mode
>>>> 9948829 [3/7] brcmfmac: cleanup brcmf_cfg80211_escan() function
>>>> 9948833 [4/7] brcmfmac: use msecs_to_jiffies() instead of calculation
>>>> using HZ
>>>> 9948827 [5/7] brcmfmac: get rid of brcmf_cfg80211_escan() function
>>>> 9948823 [6/7] brcmfmac: get rid of struct
>>>> brcmf_cfg80211_info::active_scan field
>>>> 9948835 [7/7] brcmfmac: move configuration of probe request IEs
>>>>
>>>> --
>>>> https://patchwork.kernel.org/patch/9948825/
>>>>
>>>>
>>>> https://wireless.wiki.kernel.org/en/developers/documentation/submittin=
gpatches
>>>>
>>>
>>> We (Raspberry Pi) currently have an issue open with Cypress to try and
>>> determine the cause of the firmware lockup which results in this
>>> mailbox error, we have some custom firmware that has better firmware
>>> diagnostics which we have been reporting back to Cypress. Not had any
>>> progress so far as far as I know. Does this kernel side fix help our
>>> users in any way or is it simply a better error message?
>>>
>>> James
>>
>>
>> Sorry, should have read the patch first, simply a change to the error
>> reporting. I'll try and determine how far Cypress have got with the
>> firmware.
>
>
> Hi James,
>
> This was indeed only about getting rid of the "unknown mailbox" message. =
The
> times I have seen this also brcmf_sdio_checkdied() was printing a message
> that a firmware trap occurred. Does the forensics file in debugfs show
> anything for your issue or does the probe fail resulting in a detach.
>
> Regards,
> Arend

We have a lengthy thread on our github with as much information as I
know (https://github.com/raspberrypi/linux/issues/1342). Other Pi
users seem to have more luck replicating it that I do (no change
there), although I have a rig running now waiting for it to happen. We
are fairly sure that the firmware is locking up for some reason,
probably in a high usage, low reception environment, but I am not sure
if people can recover the wireless without reboot. We have test
firmware from Cypress with more diagnostics in the probable fault area
to help track it down, previous version have provided useful
forensics, but nothing on the latest firmware so far.

James

2017-09-12 09:22:33

by Arend van Spriel

[permalink] [raw]
Subject: [PATCH 6/7] brcmfmac: get rid of struct brcmf_cfg80211_info::active_scan field

The field struct brcmf_cfg80211_info::active_scan is set to true upon
initializing the driver instance, but it is never changed so simply
get rid of it.

Reviewed-by: Hante Meuleman <[email protected]>
Reviewed-by: Pieter-Paul Giesberts <[email protected]>
Reviewed-by: Franky Lin <[email protected]>
Signed-off-by: Arend van Spriel <[email protected]>
---
drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 10 +---------
drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.h | 2 --
drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c | 5 +----
3 files changed, 2 insertions(+), 15 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index 34c2945..35e9cac 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -1054,7 +1054,6 @@ static void brcmf_escan_prep(struct brcmf_cfg80211_info *cfg,
{
struct brcmf_cfg80211_info *cfg = ifp->drvr->config;
s32 err;
- u32 passive_scan;
struct brcmf_scan_results *results;
struct escan_info *escan = &cfg->escan_info;

@@ -1062,13 +1061,7 @@ static void brcmf_escan_prep(struct brcmf_cfg80211_info *cfg,
escan->ifp = ifp;
escan->wiphy = cfg->wiphy;
escan->escan_state = WL_ESCAN_STATE_SCANNING;
- passive_scan = cfg->active_scan ? 0 : 1;
- err = brcmf_fil_cmd_int_set(ifp, BRCMF_C_SET_PASSIVE_SCAN,
- passive_scan);
- if (err) {
- brcmf_err("error (%d)\n", err);
- return err;
- }
+
brcmf_scan_config_mpc(ifp, 0);
results = (struct brcmf_scan_results *)cfg->escan_info.escan_buf;
results->version = 0;
@@ -5767,7 +5760,6 @@ static s32 wl_init_priv(struct brcmf_cfg80211_info *cfg)

cfg->scan_request = NULL;
cfg->pwr_save = true;
- cfg->active_scan = true; /* we do active scan per default */
cfg->dongle_up = false; /* dongle is not up yet */
err = brcmf_init_priv_mem(cfg);
if (err)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.h
index 7b2835e..b5b5f0f 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.h
@@ -283,7 +283,6 @@ struct brcmf_cfg80211_wowl {
* @scan_status: scan activity on the dongle.
* @pub: common driver information.
* @channel: current channel.
- * @active_scan: current scan mode.
* @int_escan_map: bucket map for which internal e-scan is done.
* @ibss_starter: indicates this sta is ibss starter.
* @pwr_save: indicate whether dongle to support power save mode.
@@ -316,7 +315,6 @@ struct brcmf_cfg80211_info {
unsigned long scan_status;
struct brcmf_pub *pub;
u32 channel;
- bool active_scan;
u32 int_escan_map;
bool ibss_starter;
bool pwr_save;
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
index 2ce675a..8e0b3f4 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
@@ -692,10 +692,7 @@ static s32 brcmf_p2p_escan(struct brcmf_p2p_info *p2p, u32 num_chans,

/* determine the scan engine parameters */
sparams->bss_type = DOT11_BSSTYPE_ANY;
- if (p2p->cfg->active_scan)
- sparams->scan_type = 0;
- else
- sparams->scan_type = 1;
+ sparams->scan_type = BRCMF_SCANTYPE_ACTIVE;

eth_broadcast_addr(sparams->bssid);
sparams->home_time = cpu_to_le32(P2PAPI_SCAN_HOME_TIME_MS);
--
1.9.1

2017-09-12 09:22:33

by Arend van Spriel

[permalink] [raw]
Subject: [PATCH 4/7] brcmfmac: use msecs_to_jiffies() instead of calculation using HZ

Minor cleanup using provided macro to convert milliseconds interval
to jiffies in brcmf_cfg80211_escan().

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

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index 217160e..8e7717a 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -1126,8 +1126,8 @@ static void brcmf_escan_prep(struct brcmf_cfg80211_info *cfg,
goto scan_out;

/* Arm scan timeout timer */
- mod_timer(&cfg->escan_timeout, jiffies +
- BRCMF_ESCAN_TIMER_INTERVAL_MS * HZ / 1000);
+ mod_timer(&cfg->escan_timeout,
+ jiffies + msecs_to_jiffies(BRCMF_ESCAN_TIMER_INTERVAL_MS));

return 0;

--
1.9.1

2017-09-12 09:22:34

by Arend van Spriel

[permalink] [raw]
Subject: [PATCH 2/7] brcmfmac: disable packet filtering in promiscuous mode

From: Franky Lin <[email protected]>

Disable arp and nd offload to allow all packets sending to host.

Reported-by: Phil Elwell <[email protected]>
Tested-by: Phil Elwell <[email protected]>
Reviewed-by: Arend Van Spriel <[email protected]>
Signed-off-by: Franky Lin <[email protected]>
---
.../broadcom/brcm80211/brcmfmac/cfg80211.c | 41 ----------------------
.../wireless/broadcom/brcm80211/brcmfmac/core.c | 38 ++++++++++++++++++++
.../wireless/broadcom/brcm80211/brcmfmac/core.h | 1 +
3 files changed, 39 insertions(+), 41 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index aaed4ab..b249083 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -472,47 +472,6 @@ static void convert_key_from_CPU(struct brcmf_wsec_key *key,
return err;
}

-static s32
-brcmf_configure_arp_nd_offload(struct brcmf_if *ifp, bool enable)
-{
- s32 err;
- u32 mode;
-
- if (enable)
- mode = BRCMF_ARP_OL_AGENT | BRCMF_ARP_OL_PEER_AUTO_REPLY;
- else
- mode = 0;
-
- /* Try to set and enable ARP offload feature, this may fail, then it */
- /* is simply not supported and err 0 will be returned */
- err = brcmf_fil_iovar_int_set(ifp, "arp_ol", mode);
- if (err) {
- brcmf_dbg(TRACE, "failed to set ARP offload mode to 0x%x, err = %d\n",
- mode, err);
- err = 0;
- } else {
- err = brcmf_fil_iovar_int_set(ifp, "arpoe", enable);
- if (err) {
- brcmf_dbg(TRACE, "failed to configure (%d) ARP offload err = %d\n",
- enable, err);
- err = 0;
- } else
- brcmf_dbg(TRACE, "successfully configured (%d) ARP offload to 0x%x\n",
- enable, mode);
- }
-
- err = brcmf_fil_iovar_int_set(ifp, "ndoe", enable);
- if (err) {
- brcmf_dbg(TRACE, "failed to configure (%d) ND offload err = %d\n",
- enable, err);
- err = 0;
- } else
- brcmf_dbg(TRACE, "successfully configured (%d) ND offload to 0x%x\n",
- enable, mode);
-
- return err;
-}
-
static void
brcmf_cfg80211_update_proto_addr_mode(struct wireless_dev *wdev)
{
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
index 5cc3a07..9c7536d 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
@@ -71,6 +71,43 @@ struct brcmf_if *brcmf_get_ifp(struct brcmf_pub *drvr, int ifidx)
return ifp;
}

+void brcmf_configure_arp_nd_offload(struct brcmf_if *ifp, bool enable)
+{
+ s32 err;
+ u32 mode;
+
+ if (enable)
+ mode = BRCMF_ARP_OL_AGENT | BRCMF_ARP_OL_PEER_AUTO_REPLY;
+ else
+ mode = 0;
+
+ /* Try to set and enable ARP offload feature, this may fail, then it */
+ /* is simply not supported and err 0 will be returned */
+ err = brcmf_fil_iovar_int_set(ifp, "arp_ol", mode);
+ if (err) {
+ brcmf_dbg(TRACE, "failed to set ARP offload mode to 0x%x, err = %d\n",
+ mode, err);
+ } else {
+ err = brcmf_fil_iovar_int_set(ifp, "arpoe", enable);
+ if (err) {
+ brcmf_dbg(TRACE, "failed to configure (%d) ARP offload err = %d\n",
+ enable, err);
+ } else {
+ brcmf_dbg(TRACE, "successfully configured (%d) ARP offload to 0x%x\n",
+ enable, mode);
+ }
+ }
+
+ err = brcmf_fil_iovar_int_set(ifp, "ndoe", enable);
+ if (err) {
+ brcmf_dbg(TRACE, "failed to configure (%d) ND offload err = %d\n",
+ enable, err);
+ } else {
+ brcmf_dbg(TRACE, "successfully configured (%d) ND offload to 0x%x\n",
+ enable, mode);
+ }
+}
+
static void _brcmf_set_multicast_list(struct work_struct *work)
{
struct brcmf_if *ifp;
@@ -134,6 +171,7 @@ static void _brcmf_set_multicast_list(struct work_struct *work)
if (err < 0)
brcmf_err("Setting BRCMF_C_SET_PROMISC failed, %d\n",
err);
+ brcmf_configure_arp_nd_offload(ifp, !cmd_value);
}

#if IS_ENABLED(CONFIG_IPV6)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h
index a4dd313..1708571 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h
@@ -203,6 +203,7 @@ struct brcmf_if {
/* Return pointer to interface name */
char *brcmf_ifname(struct brcmf_if *ifp);
struct brcmf_if *brcmf_get_ifp(struct brcmf_pub *drvr, int ifidx);
+void brcmf_configure_arp_nd_offload(struct brcmf_if *ifp, bool enable);
int brcmf_net_attach(struct brcmf_if *ifp, bool rtnl_locked);
struct brcmf_if *brcmf_add_if(struct brcmf_pub *drvr, s32 bsscfgidx, s32 ifidx,
bool is_p2pdev, const char *name, u8 *mac_addr);
--
1.9.1

2017-09-26 11:11:08

by Arend van Spriel

[permalink] [raw]
Subject: Re: [1/7] brcmfmac: handle FWHALT mailbox indication

On 25-09-17 11:30, James Hughes wrote:
> On 25 September 2017 at 10:26, James Hughes
> <[email protected]> wrote:
>> On 25 September 2017 at 09:18, Kalle Valo <[email protected]> wrote:
>>> Arend Van Spriel <[email protected]> wrote:
>>>
>>>> The firmware uses a mailbox to communicate to the host what is going
>>>> on. In the driver we validate the bit received. Various people seen
>>>> the following message:
>>>>
>>>> brcmfmac: brcmf_sdio_hostmail: Unknown mailbox data content: 0x40012
>>>>
>>>> Bit 4 is cause of this message, but this actually indicates the firmware
>>>> has halted. Handle this bit by giving a more meaningful error message.
>>>>
>>>> Reviewed-by: Hante Meuleman <[email protected]>
>>>> Reviewed-by: Pieter-Paul Giesberts <[email protected]>
>>>> Reviewed-by: Franky Lin <[email protected]>
>>>> Signed-off-by: Arend van Spriel <[email protected]>
>>>
>>> Failed to compile:
>>>
>>> drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c: In function ‘brcmf_p2p_escan’:
>>> drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c:695:23: error: ‘BRCMF_SCANTYPE_ACTIVE’ undeclared (first use in this function)
>>> sparams->scan_type = BRCMF_SCANTYPE_ACTIVE;
>>> ^
>>> drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c:695:23: note: each undeclared identifier is reported only once for each function it appears in
>>> make[6]: *** [drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.o] Error 1
>>> make[6]: *** Waiting for unfinished jobs....
>>> make[5]: *** [drivers/net/wireless/broadcom/brcm80211/brcmfmac] Error 2
>>> make[4]: *** [drivers/net/wireless/broadcom/brcm80211] Error 2
>>> make[3]: *** [drivers/net/wireless/broadcom] Error 2
>>> make[2]: *** [drivers/net/wireless] Error 2
>>> make[1]: *** [drivers/net] Error 2
>>> make: *** [drivers] Error 2
>>>
>>> 7 patches set to Changes Requested.
>>>
>>> 9948825 [1/7] brcmfmac: handle FWHALT mailbox indication
>>> 9948831 [2/7] brcmfmac: disable packet filtering in promiscuous mode
>>> 9948829 [3/7] brcmfmac: cleanup brcmf_cfg80211_escan() function
>>> 9948833 [4/7] brcmfmac: use msecs_to_jiffies() instead of calculation using HZ
>>> 9948827 [5/7] brcmfmac: get rid of brcmf_cfg80211_escan() function
>>> 9948823 [6/7] brcmfmac: get rid of struct brcmf_cfg80211_info::active_scan field
>>> 9948835 [7/7] brcmfmac: move configuration of probe request IEs
>>>
>>> --
>>> https://patchwork.kernel.org/patch/9948825/
>>>
>>> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
>>>
>>
>> We (Raspberry Pi) currently have an issue open with Cypress to try and
>> determine the cause of the firmware lockup which results in this
>> mailbox error, we have some custom firmware that has better firmware
>> diagnostics which we have been reporting back to Cypress. Not had any
>> progress so far as far as I know. Does this kernel side fix help our
>> users in any way or is it simply a better error message?
>>
>> James
>
> Sorry, should have read the patch first, simply a change to the error
> reporting. I'll try and determine how far Cypress have got with the
> firmware.

Hi James,

This was indeed only about getting rid of the "unknown mailbox" message.
The times I have seen this also brcmf_sdio_checkdied() was printing a
message that a firmware trap occurred. Does the forensics file in
debugfs show anything for your issue or does the probe fail resulting in
a detach.

Regards,
Arend

2017-09-25 09:26:30

by James Hughes

[permalink] [raw]
Subject: Re: [1/7] brcmfmac: handle FWHALT mailbox indication

On 25 September 2017 at 09:18, Kalle Valo <[email protected]> wrote:
> Arend Van Spriel <[email protected]> wrote:
>
>> The firmware uses a mailbox to communicate to the host what is going
>> on. In the driver we validate the bit received. Various people seen
>> the following message:
>>
>> brcmfmac: brcmf_sdio_hostmail: Unknown mailbox data content: 0x40012
>>
>> Bit 4 is cause of this message, but this actually indicates the firmware
>> has halted. Handle this bit by giving a more meaningful error message.
>>
>> Reviewed-by: Hante Meuleman <[email protected]>
>> Reviewed-by: Pieter-Paul Giesberts <[email protected]>
>> Reviewed-by: Franky Lin <[email protected]>
>> Signed-off-by: Arend van Spriel <[email protected]>
>
> Failed to compile:
>
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c: In function =E2=
=80=98brcmf_p2p_escan=E2=80=99:
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c:695:23: error: =E2=
=80=98BRCMF_SCANTYPE_ACTIVE=E2=80=99 undeclared (first use in this function=
)
> sparams->scan_type =3D BRCMF_SCANTYPE_ACTIVE;
> ^
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c:695:23: note: each=
undeclared identifier is reported only once for each function it appears i=
n
> make[6]: *** [drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.o] Err=
or 1
> make[6]: *** Waiting for unfinished jobs....
> make[5]: *** [drivers/net/wireless/broadcom/brcm80211/brcmfmac] Error 2
> make[4]: *** [drivers/net/wireless/broadcom/brcm80211] Error 2
> make[3]: *** [drivers/net/wireless/broadcom] Error 2
> make[2]: *** [drivers/net/wireless] Error 2
> make[1]: *** [drivers/net] Error 2
> make: *** [drivers] Error 2
>
> 7 patches set to Changes Requested.
>
> 9948825 [1/7] brcmfmac: handle FWHALT mailbox indication
> 9948831 [2/7] brcmfmac: disable packet filtering in promiscuous mode
> 9948829 [3/7] brcmfmac: cleanup brcmf_cfg80211_escan() function
> 9948833 [4/7] brcmfmac: use msecs_to_jiffies() instead of calculation usi=
ng HZ
> 9948827 [5/7] brcmfmac: get rid of brcmf_cfg80211_escan() function
> 9948823 [6/7] brcmfmac: get rid of struct brcmf_cfg80211_info::active_sca=
n field
> 9948835 [7/7] brcmfmac: move configuration of probe request IEs
>
> --
> https://patchwork.kernel.org/patch/9948825/
>
> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpa=
tches
>

We (Raspberry Pi) currently have an issue open with Cypress to try and
determine the cause of the firmware lockup which results in this
mailbox error, we have some custom firmware that has better firmware
diagnostics which we have been reporting back to Cypress. Not had any
progress so far as far as I know. Does this kernel side fix help our
users in any way or is it simply a better error message?

James

2017-09-12 09:22:34

by Arend van Spriel

[permalink] [raw]
Subject: [PATCH 3/7] brcmfmac: cleanup brcmf_cfg80211_escan() function

The function brcmf_cfg80211_escan() was always called with a non-null
request parameter and null pointer for this_ssid parameter. Clean up
the function removing the dead code path.

Reviewed-by: Hante Meuleman <[email protected]>
Reviewed-by: Pieter-Paul Giesberts <[email protected]>
Reviewed-by: Franky Lin <[email protected]>
Signed-off-by: Arend van Spriel <[email protected]>
---
.../broadcom/brcm80211/brcmfmac/cfg80211.c | 76 ++++------------------
1 file changed, 11 insertions(+), 65 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index b249083..217160e 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -1083,18 +1083,10 @@ static void brcmf_escan_prep(struct brcmf_cfg80211_info *cfg,

static s32
brcmf_cfg80211_escan(struct wiphy *wiphy, struct brcmf_cfg80211_vif *vif,
- struct cfg80211_scan_request *request,
- struct cfg80211_ssid *this_ssid)
+ struct cfg80211_scan_request *request)
{
- struct brcmf_if *ifp = vif->ifp;
struct brcmf_cfg80211_info *cfg = wiphy_to_cfg(wiphy);
- struct cfg80211_ssid *ssids;
- u32 passive_scan;
- bool escan_req;
- bool spec_scan;
s32 err;
- struct brcmf_ssid_le ssid_le;
- u32 SSID_len;

brcmf_dbg(SCAN, "START ESCAN\n");

@@ -1112,8 +1104,8 @@ static void brcmf_escan_prep(struct brcmf_cfg80211_info *cfg,
cfg->scan_status);
return -EAGAIN;
}
- if (test_bit(BRCMF_VIF_STATUS_CONNECTING, &ifp->vif->sme_state)) {
- brcmf_err("Connecting: status (%lu)\n", ifp->vif->sme_state);
+ if (test_bit(BRCMF_VIF_STATUS_CONNECTING, &vif->sme_state)) {
+ brcmf_err("Connecting: status (%lu)\n", vif->sme_state);
return -EAGAIN;
}

@@ -1121,63 +1113,17 @@ static void brcmf_escan_prep(struct brcmf_cfg80211_info *cfg,
if (vif == cfg->p2p.bss_idx[P2PAPI_BSSCFG_DEVICE].vif)
vif = cfg->p2p.bss_idx[P2PAPI_BSSCFG_PRIMARY].vif;

- escan_req = false;
- if (request) {
- /* scan bss */
- ssids = request->ssids;
- escan_req = true;
- } else {
- /* scan in ibss */
- /* we don't do escan in ibss */
- ssids = this_ssid;
- }
-
cfg->scan_request = request;
set_bit(BRCMF_SCAN_STATUS_BUSY, &cfg->scan_status);
- if (escan_req) {
- cfg->escan_info.run = brcmf_run_escan;
- err = brcmf_p2p_scan_prep(wiphy, request, vif);
- if (err)
- goto scan_out;
-
- err = brcmf_do_escan(vif->ifp, request);
- if (err)
- goto scan_out;
- } else {
- brcmf_dbg(SCAN, "ssid \"%s\", ssid_len (%d)\n",
- ssids->ssid, ssids->ssid_len);
- memset(&ssid_le, 0, sizeof(ssid_le));
- SSID_len = min_t(u8, sizeof(ssid_le.SSID), ssids->ssid_len);
- ssid_le.SSID_len = cpu_to_le32(0);
- spec_scan = false;
- if (SSID_len) {
- memcpy(ssid_le.SSID, ssids->ssid, SSID_len);
- ssid_le.SSID_len = cpu_to_le32(SSID_len);
- spec_scan = true;
- } else
- brcmf_dbg(SCAN, "Broadcast scan\n");

- passive_scan = cfg->active_scan ? 0 : 1;
- err = brcmf_fil_cmd_int_set(ifp, BRCMF_C_SET_PASSIVE_SCAN,
- passive_scan);
- if (err) {
- brcmf_err("WLC_SET_PASSIVE_SCAN error (%d)\n", err);
- goto scan_out;
- }
- brcmf_scan_config_mpc(ifp, 0);
- err = brcmf_fil_cmd_data_set(ifp, BRCMF_C_SCAN, &ssid_le,
- sizeof(ssid_le));
- if (err) {
- if (err == -EBUSY)
- brcmf_dbg(INFO, "BUSY: scan for \"%s\" canceled\n",
- ssid_le.SSID);
- else
- brcmf_err("WLC_SCAN error (%d)\n", err);
+ cfg->escan_info.run = brcmf_run_escan;
+ err = brcmf_p2p_scan_prep(wiphy, request, vif);
+ if (err)
+ goto scan_out;

- brcmf_scan_config_mpc(ifp, 1);
- goto scan_out;
- }
- }
+ err = brcmf_do_escan(vif->ifp, request);
+ if (err)
+ goto scan_out;

/* Arm scan timeout timer */
mod_timer(&cfg->escan_timeout, jiffies +
@@ -1202,7 +1148,7 @@ static void brcmf_escan_prep(struct brcmf_cfg80211_info *cfg,
if (!check_vif_up(vif))
return -EIO;

- err = brcmf_cfg80211_escan(wiphy, vif, request, NULL);
+ err = brcmf_cfg80211_escan(wiphy, vif, request);

if (err)
brcmf_err("scan error (%d)\n", err);
--
1.9.1

2017-09-12 09:22:34

by Arend van Spriel

[permalink] [raw]
Subject: [PATCH 7/7] brcmfmac: move configuration of probe request IEs

The configuration of the IEs for probe requests was done in a P2P
related function, which is not very obvious. Moving it to
.scan callback function, ie. brcmf_cfg80211_scan().

Reviewed-by: Hante Meuleman <[email protected]>
Reviewed-by: Pieter-Paul Giesberts <[email protected]>
Reviewed-by: Franky Lin <[email protected]>
Signed-off-by: Arend van Spriel <[email protected]>
---
drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 5 +++++
drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c | 6 ++----
2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index 35e9cac..b6d1aaa 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -1119,6 +1119,11 @@ static void brcmf_escan_prep(struct brcmf_cfg80211_info *cfg,
if (err)
goto scan_out;

+ err = brcmf_vif_set_mgmt_ie(vif, BRCMF_VNDR_IE_PRBREQ_FLAG,
+ request->ie, request->ie_len);
+ if (err)
+ goto scan_out;
+
err = brcmf_do_escan(vif->ifp, request);
if (err)
goto scan_out;
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
index 8e0b3f4..cec25dd 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
@@ -881,7 +881,7 @@ int brcmf_p2p_scan_prep(struct wiphy *wiphy,
{
struct brcmf_cfg80211_info *cfg = wiphy_to_cfg(wiphy);
struct brcmf_p2p_info *p2p = &cfg->p2p;
- int err = 0;
+ int err;

if (brcmf_p2p_scan_is_p2p_request(request)) {
/* find my listen channel */
@@ -904,9 +904,7 @@ int brcmf_p2p_scan_prep(struct wiphy *wiphy,
/* override .run_escan() callback. */
cfg->escan_info.run = brcmf_p2p_run_escan;
}
- err = brcmf_vif_set_mgmt_ie(vif, BRCMF_VNDR_IE_PRBREQ_FLAG,
- request->ie, request->ie_len);
- return err;
+ return 0;
}


--
1.9.1

2017-09-25 09:30:41

by James Hughes

[permalink] [raw]
Subject: Re: [1/7] brcmfmac: handle FWHALT mailbox indication

On 25 September 2017 at 10:26, James Hughes
<[email protected]> wrote:
> On 25 September 2017 at 09:18, Kalle Valo <[email protected]> wrote:
>> Arend Van Spriel <[email protected]> wrote:
>>
>>> The firmware uses a mailbox to communicate to the host what is going
>>> on. In the driver we validate the bit received. Various people seen
>>> the following message:
>>>
>>> brcmfmac: brcmf_sdio_hostmail: Unknown mailbox data content: 0x40012
>>>
>>> Bit 4 is cause of this message, but this actually indicates the firmwar=
e
>>> has halted. Handle this bit by giving a more meaningful error message.
>>>
>>> Reviewed-by: Hante Meuleman <[email protected]>
>>> Reviewed-by: Pieter-Paul Giesberts <[email protected]>
>>> Reviewed-by: Franky Lin <[email protected]>
>>> Signed-off-by: Arend van Spriel <[email protected]>
>>
>> Failed to compile:
>>
>> drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c: In function =E2=
=80=98brcmf_p2p_escan=E2=80=99:
>> drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c:695:23: error: =
=E2=80=98BRCMF_SCANTYPE_ACTIVE=E2=80=99 undeclared (first use in this funct=
ion)
>> sparams->scan_type =3D BRCMF_SCANTYPE_ACTIVE;
>> ^
>> drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c:695:23: note: eac=
h undeclared identifier is reported only once for each function it appears =
in
>> make[6]: *** [drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.o] Er=
ror 1
>> make[6]: *** Waiting for unfinished jobs....
>> make[5]: *** [drivers/net/wireless/broadcom/brcm80211/brcmfmac] Error 2
>> make[4]: *** [drivers/net/wireless/broadcom/brcm80211] Error 2
>> make[3]: *** [drivers/net/wireless/broadcom] Error 2
>> make[2]: *** [drivers/net/wireless] Error 2
>> make[1]: *** [drivers/net] Error 2
>> make: *** [drivers] Error 2
>>
>> 7 patches set to Changes Requested.
>>
>> 9948825 [1/7] brcmfmac: handle FWHALT mailbox indication
>> 9948831 [2/7] brcmfmac: disable packet filtering in promiscuous mode
>> 9948829 [3/7] brcmfmac: cleanup brcmf_cfg80211_escan() function
>> 9948833 [4/7] brcmfmac: use msecs_to_jiffies() instead of calculation us=
ing HZ
>> 9948827 [5/7] brcmfmac: get rid of brcmf_cfg80211_escan() function
>> 9948823 [6/7] brcmfmac: get rid of struct brcmf_cfg80211_info::active_sc=
an field
>> 9948835 [7/7] brcmfmac: move configuration of probe request IEs
>>
>> --
>> https://patchwork.kernel.org/patch/9948825/
>>
>> https://wireless.wiki.kernel.org/en/developers/documentation/submittingp=
atches
>>
>
> We (Raspberry Pi) currently have an issue open with Cypress to try and
> determine the cause of the firmware lockup which results in this
> mailbox error, we have some custom firmware that has better firmware
> diagnostics which we have been reporting back to Cypress. Not had any
> progress so far as far as I know. Does this kernel side fix help our
> users in any way or is it simply a better error message?
>
> James

Sorry, should have read the patch first, simply a change to the error
reporting. I'll try and determine how far Cypress have got with the
firmware.

2017-09-25 08:18:17

by Kalle Valo

[permalink] [raw]
Subject: Re: [1/7] brcmfmac: handle FWHALT mailbox indication

Arend Van Spriel <[email protected]> wrote:

> The firmware uses a mailbox to communicate to the host what is going
> on. In the driver we validate the bit received. Various people seen
> the following message:
>
> brcmfmac: brcmf_sdio_hostmail: Unknown mailbox data content: 0x40012
>
> Bit 4 is cause of this message, but this actually indicates the firmware
> has halted. Handle this bit by giving a more meaningful error message.
>
> Reviewed-by: Hante Meuleman <[email protected]>
> Reviewed-by: Pieter-Paul Giesberts <[email protected]>
> Reviewed-by: Franky Lin <[email protected]>
> Signed-off-by: Arend van Spriel <[email protected]>

Failed to compile:

drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c: In function ‘brcmf_p2p_escan’:
drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c:695:23: error: ‘BRCMF_SCANTYPE_ACTIVE’ undeclared (first use in this function)
sparams->scan_type = BRCMF_SCANTYPE_ACTIVE;
^
drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c:695:23: note: each undeclared identifier is reported only once for each function it appears in
make[6]: *** [drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.o] Error 1
make[6]: *** Waiting for unfinished jobs....
make[5]: *** [drivers/net/wireless/broadcom/brcm80211/brcmfmac] Error 2
make[4]: *** [drivers/net/wireless/broadcom/brcm80211] Error 2
make[3]: *** [drivers/net/wireless/broadcom] Error 2
make[2]: *** [drivers/net/wireless] Error 2
make[1]: *** [drivers/net] Error 2
make: *** [drivers] Error 2

7 patches set to Changes Requested.

9948825 [1/7] brcmfmac: handle FWHALT mailbox indication
9948831 [2/7] brcmfmac: disable packet filtering in promiscuous mode
9948829 [3/7] brcmfmac: cleanup brcmf_cfg80211_escan() function
9948833 [4/7] brcmfmac: use msecs_to_jiffies() instead of calculation using HZ
9948827 [5/7] brcmfmac: get rid of brcmf_cfg80211_escan() function
9948823 [6/7] brcmfmac: get rid of struct brcmf_cfg80211_info::active_scan field
9948835 [7/7] brcmfmac: move configuration of probe request IEs

--
https://patchwork.kernel.org/patch/9948825/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches