RE: [1/7] brcmfmac: handle FWHALT mailbox indication

RE: [1/7] brcmfmac: handle FWHALT mailbox indication

[Arend van Spriel]

Hi Kalle,

I was preparing a new patch series and got a bit confused. Turns out 
there was a series still pending in patchwork.

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

So I applied the pending series on wireless-drivers-next and it compiles 
fine. So what I think happened is that the series depended on a patch 
that was submitted to wireless-drivers at that time. Can these patches 
be taken now or do I need to resubmit.

Regards,
Arend

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

[Kalle Valo]

Arend van Spriel <arend.vanspriel@broadcom.com> writes:

> Hi Kalle,
>
> I was preparing a new patch series and got a bit confused. Turns out
> there was a series still pending in patchwork.
>
> https://patchwork.kernel.org/patch/9948825/

That's not pending, it's in "Changes Requested" meaning that I have
dropped them:

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

"Deferred" is the state when patch is pending an action from me. Yes,
patchwork state names suck :)

> So I applied the pending series on wireless-drivers-next and it
> compiles fine. So what I think happened is that the series depended on
> a patch that was submitted to wireless-drivers at that time. Can these
> patches be taken now or do I need to resubmit.

To keep things simple I would prefer you resubmit them. I'm at Netdev
2.2 right now so on a quite tight schedule to get everything ready
before the weekend when the merge window opens.

-- 
Kalle Valo

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

[Arend van Spriel]

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 <hante.meuleman@broadcom.com>
Reviewed-by: Pieter-Paul Giesberts <pieter-paul.giesberts@broadcom.com>
Reviewed-by: Franky Lin <franky.lin@broadcom.com>
Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
---
 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

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

[Kalle Valo]

Arend Van Spriel <arend.vanspriel@broadcom.com> 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 <hante.meuleman@broadcom.com>
> Reviewed-by: Pieter-Paul Giesberts <pieter-paul.giesberts@broadcom.com>
> Reviewed-by: Franky Lin <franky.lin@broadcom.com>
> Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>

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

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

[James Hughes]

On 25 September 2017 at 09:18, Kalle Valo <kvalo@codeaurora.org> wrote:
> Arend Van Spriel <arend.vanspriel@broadcom.com> 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 <hante.meuleman@broadcom.com>
>> Reviewed-by: Pieter-Paul Giesberts <pieter-paul.giesberts@broadcom.com>
>> Reviewed-by: Franky Lin <franky.lin@broadcom.com>
>> Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
>
> 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

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

[James Hughes]

On 25 September 2017 at 10:26, James Hughes
<james.hughes@raspberrypi.org> wrote:
> On 25 September 2017 at 09:18, Kalle Valo <kvalo@codeaurora.org> wrote:
>> Arend Van Spriel <arend.vanspriel@broadcom.com> 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 <hante.meuleman@broadcom.com>
>>> Reviewed-by: Pieter-Paul Giesberts <pieter-paul.giesberts@broadcom.com>
>>> Reviewed-by: Franky Lin <franky.lin@broadcom.com>
>>> Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
>>
>> 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.

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

[Arend van Spriel]

On 25-09-17 11:30, James Hughes wrote:
> On 25 September 2017 at 10:26, James Hughes
> <james.hughes@raspberrypi.org> wrote:
>> On 25 September 2017 at 09:18, Kalle Valo <kvalo@codeaurora.org> wrote:
>>> Arend Van Spriel <arend.vanspriel@broadcom.com> 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 <hante.meuleman@broadcom.com>
>>>> Reviewed-by: Pieter-Paul Giesberts <pieter-paul.giesberts@broadcom.com>
>>>> Reviewed-by: Franky Lin <franky.lin@broadcom.com>
>>>> Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
>>>
>>> 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

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

[James Hughes]

On 26 September 2017 at 12:11, Arend van Spriel
<arend.vanspriel@broadcom.com> wrote:
> On 25-09-17 11:30, James Hughes wrote:
>>
>> On 25 September 2017 at 10:26, James Hughes
>> <james.hughes@raspberrypi.org> wrote:
>>>
>>> On 25 September 2017 at 09:18, Kalle Valo <kvalo@codeaurora.org> wrote:
>>>>
>>>> Arend Van Spriel <arend.vanspriel@broadcom.com> 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 <hante.meuleman@broadcom.com>
>>>>> Reviewed-by: Pieter-Paul Giesberts <pieter-paul.giesberts@broadcom.co=
m>
>>>>> Reviewed-by: Franky Lin <franky.lin@broadcom.com>
>>>>> Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
>>>>
>>>>
>>>> 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