brcmfmac: PCIE debug mechanism series

Add several debug mechanism on PCIE operation.

Wright Feng (3):
brcmfmac: add a timer to read console periodically in PCIE bus
brcmfmac: return error when getting invalid max_flowrings from dongle
brcmfmac: dump dongle memory when attaching failed

 .../broadcom/brcm80211/brcmfmac/bus.h         |   6 +
 .../broadcom/brcm80211/brcmfmac/core.c        |   3 +-
 .../broadcom/brcm80211/brcmfmac/pcie.c        | 139 +++++++++++++++++-
 .../broadcom/brcm80211/brcmfmac/sdio.c        |   2 -
 4 files changed, 144 insertions(+), 6 deletions(-)

2.25.0

From: Wright Feng <[email protected]>

Currently, host only reads console buffer when receiving mailbox data or hit crash with PCIE bus. Therefore, we add timer in PCIE code to read console buffer periodically to help developer and user check firmware message when there is no data transmission between host and dongle.

---
 .../broadcom/brcm80211/brcmfmac/bus.h         |   6 +
 .../broadcom/brcm80211/brcmfmac/pcie.c        | 125 ++++++++++++++++++
 .../broadcom/brcm80211/brcmfmac/sdio.c        |   2 -
 3 files changed, 131 insertions(+), 2 deletions(-)

index 2208ab3aa795..60f5645aead3 100644
@@ -24,6 +24,12 @@
 #define BRCMF_NROF_COMMON_MSGRINGS	(BRCMF_NROF_H2D_COMMON_MSGRINGS + \
 					 BRCMF_NROF_D2H_COMMON_MSGRINGS)
 
+/* The interval to poll console */
+#define BRCMF_CONSOLE	10
+
+/* The maximum console interval value (5 mins) */
+#define MAX_CONSOLE_INTERVAL	(5 * 60)
+
 /* The level of bus communication with the dongle */
 enum brcmf_bus_state {
 	BRCMF_BUS_DOWN,		/* Not ready for frame transfers */

index 465d95d83759..deb0199fcf8c 100644
@@ -135,8 +135,6 @@ struct rte_console {
 
 #define BRCMF_FIRSTREAD	(1 << 6)
 
-#define BRCMF_CONSOLE	10	/* watchdog interval to poll console */
-
 /* SBSDIO_DEVICE_CTL */
 
 /* 1: device will assert busy signal when receiving CMD53 */

2.25.0

[...]

https://patchwork.kernel.org/project/linux-wireless/list/

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

3 patches applied to wireless-next.git, thanks.

dcb485dfc83b brcmfmac: add a timer to read console periodically in PCIE bus 2aca4f3734bd brcmfmac: return error when getting invalid max_flowrings from dongle 5671c8b56c32 brcmfmac: dump dongle memory when attaching failed

https://patchwork.kernel.org/project/linux-wireless/patch/[email protected]/

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

These patches are already applied, but wanted to respond still. I know the proprietary driver does use a timer, but it was a design decision to drop that. It adds overhead and complexity which is not resulting in more information about the firmware. Maybe I am missing the exact scenarios in which this would be useful. Also description above says it would help the user, but I very much doubt that. Not much users are fiddling with debugfs and if they are we can call them developers ;-)

Regards,
Arend

From: Wright Feng <[email protected]>

When firmware hit trap at initialization, host will read abnormal max_flowrings number from dongle, and it will cause kernel panic when doing iowrite to initialize dongle ring.
To detect this error at early stage, we directly return error when getting invalid max_flowrings(>256).

---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c | 4 ++++
 1 file changed, 4 insertions(+)

index 2b7ebbd7b5b4..1becd50038ab 100644
@@ -1228,6 +1228,10 @@ static int brcmf_pcie_init_ringbuffers(struct brcmf_pciedev_info *devinfo)
 				BRCMF_NROF_H2D_COMMON_MSGRINGS;
 		max_completionrings = BRCMF_NROF_D2H_COMMON_MSGRINGS;
 	}
+	if (max_flowrings > 256) {
+		brcmf_err(bus, "invalid max_flowrings(%d)\n", max_flowrings);
+		return -EIO;
+	}
 
 	if (devinfo->dma_idx_sz != 0) {
 		bufsz = (max_submissionrings + max_completionrings) *

2.25.0

Hi! This patch:
https://lore.kernel.org/all/[email protected]/

causes my Asus PCE-AC88 (1043:86fb) to stop working, because when modprobe loads brcmfmac it throws this error:
---

[ 2740.647600] brcmfmac 0000:0d:00.0: brcmf_pcie_init_ringbuffers: invalid 

max_flowrings(264).
---

This is also being reported on reddit:
https://www.reddit.com/r/archlinux/comments/104pqv9/
no_wifi_since_kernel_update_yesterday/

(also, see https://forum.manjaro.org/t/brcmfmac-fails-to-load/131128)

I think that the check on value > 256 is used as a kind of red herring to indirectly spot an inconsistent properties report from the PCI bus (?) about the network card.
Before that patch, the driver was correctly working for me and I could not observe any kernel panic. Maybe, at least in my case, the wrong value was still reported but could be safely ignored, as at a later point in time, when the card is initialised, everything worked fine.

I am now stuck at kernel 6.1.1, if I upgrade I cannot use the card. Is there anything I can do to circumvent this issue? May I help somehow with debugging & testing? Please let me know...

Thanks,

(c)

The firmware loaded into the wifi device requests the host for 264 flowrings. This was apparently not expected to be a valid value, but your device shows it is.

6.2 is not formally released so I suppose you are an early adopter and aware of the risk that implies ;-) There is no real debugging needed. I will figure out what a sensible value would be and come up with a patch. If you can test that that would be great. Will let you know.

Regards,
Arend

I am on 6.1. Version 6.1.1 works, 6.1.3 does not.
Will this patch be provided with 6.1.4? Or shall I wait until 6.2.0?

Sure! :)

From: Wright Feng <[email protected]>

To enhance FW debugging, we add dongle memory dump when hitting attaching failure with PCIE bus. It can help developer to get more information about dongle trap reason and root cause.

---
 .../net/wireless/broadcom/brcm80211/brcmfmac/core.c    |  3 ++-
 .../net/wireless/broadcom/brcm80211/brcmfmac/pcie.c    | 10 +++++++---
 2 files changed, 9 insertions(+), 4 deletions(-)

index 595ae3ae561e..d354f79fd0ac 100644
@@ -1399,7 +1399,8 @@ void brcmf_fw_crashed(struct device *dev)
 
 	brcmf_dev_coredump(dev);
 
-	schedule_work(&drvr->bus_reset);
+	if (drvr->bus_reset.func)
+		schedule_work(&drvr->bus_reset);
 }
 
 void brcmf_detach(struct device *dev)

index 1becd50038ab..cf564adc612a 100644
@@ -2068,13 +2068,14 @@ static void brcmf_pcie_setup(struct device *dev, int ret,
 	struct brcmf_commonring **flowrings;
 	u32 i, nvram_len;
 
+	bus = dev_get_drvdata(dev);
+	pcie_bus_dev = bus->bus_priv.pcie;
+	devinfo = pcie_bus_dev->devinfo;
+
 	/* check firmware loading result */
 	if (ret)
 		goto fail;
 
-	bus = dev_get_drvdata(dev);
-	pcie_bus_dev = bus->bus_priv.pcie;
-	devinfo = pcie_bus_dev->devinfo;
 	brcmf_pcie_attach(devinfo);
 
 	fw = fwreq->items[BRCMF_PCIE_FW_CODE].binary;

@@ -2148,6 +2149,9 @@ static void brcmf_pcie_setup(struct device *dev, int ret,
 	return;
 
 fail:
+	brcmf_err(bus, "Dongle setup failed\n");
+	brcmf_pcie_bus_console_read(devinfo, true);
+	brcmf_fw_crashed(dev);
 	device_release_driver(dev);
 }
 

2.25.0