Skip to contentSkip to conversation

mt76: mt7915: add debugging for MCU command timeouts.

PATCH In review [email protected] 3 participants 3 messages Oct 11, 2021 – Oct 12, 2021

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 mt76: mt7915: add debugging for MCU command timeouts.

From: Ben Greear <[email protected]>

Print information about whether the message is the first timeout, and also print info if we manage to recover after a timeout.

---

NOTE: Not sure this is really something folks would want upstream, but maybe it helps someone else trying to debug this driver.

 drivers/net/wireless/mediatek/mt76/mt76.h     |  2 ++
 .../net/wireless/mediatek/mt76/mt7915/mcu.c   | 35 +++++++++++++++++--
 2 files changed, 34 insertions(+), 3 deletions(-)
drivers/net/wireless/mediatek/mt76/mt76.h +2 -0
index 2e1a1004ed30..3b64d288c7c2 100644
@@ -686,6 +686,8 @@ struct mt76_dev {
 	struct device *dev;
 
 	struct mt76_mcu mcu;
+	u32 first_failed_mcu_cmd; /* for debugging */
+	u32 last_successful_mcu_cmd; /* for debugging */
 
 	struct net_device napi_dev;
 	struct net_device tx_napi_dev;
drivers/net/wireless/mediatek/mt76/mt7915/mcu.c +32 -3
index 51241b443cc5..e97cdeae785f 100644
@@ -323,14 +323,43 @@ mt7915_mcu_parse_response(struct mt76_dev *mdev, int cmd,
 	int ret = 0;
 
 	if (!skb) {
-		dev_err(mdev->dev, "Message %08x (seq %d) timeout\n",
-			cmd, seq);
+		const char* first = "Secondary";
+
+		if (!mdev->first_failed_mcu_cmd)
+			first = "Initial";
+
+		dev_err(mdev->dev, "MCU: %s Failure: Message %08x (cid %lx ext_cid: %lx seq %d) timeout.  Last successful cmd: 0x%x\n",
+			first,
+			cmd, FIELD_GET(__MCU_CMD_FIELD_ID, cmd),
+			FIELD_GET(__MCU_CMD_FIELD_EXT_ID, cmd), seq,
+			mdev->last_successful_mcu_cmd);
+
+		if (!mdev->first_failed_mcu_cmd)
+			mdev->first_failed_mcu_cmd = cmd;
 		return -ETIMEDOUT;
 	}
 
+	mdev->last_successful_mcu_cmd = cmd;
+
+	if (mdev->first_failed_mcu_cmd) {
+		dev_err(mdev->dev, "MCU: First success after failure: Message %08x (cid %lx ext_cid: %lx seq %d)\n",
+			cmd, FIELD_GET(__MCU_CMD_FIELD_ID, cmd),
+			FIELD_GET(__MCU_CMD_FIELD_EXT_ID, cmd), seq);
+		mdev->first_failed_mcu_cmd = 0;
+	}
+	else {
+		// verbose debugging
+		//dev_err(mdev->dev, "MCU: OK response to message %08x (cid %lx ext_cid: %lx seq %d)\n",
+		//	cmd, FIELD_GET(__MCU_CMD_FIELD_ID, cmd),
+		//	FIELD_GET(__MCU_CMD_FIELD_EXT_ID, cmd), seq);
+	}
+
 	rxd = (struct mt7915_mcu_rxd *)skb->data;
-	if (seq != rxd->seq)
+	if (seq != rxd->seq) {
+		dev_err(mdev->dev, "ERROR: MCU:  Sequence mismatch in response, seq: %d  rxd->seq: %d cmd: %0x\n",
+			seq, rxd->seq, cmd);
 		return -EAGAIN;
+	}
 
 	if (cmd == MCU_CMD(PATCH_SEM_CONTROL)) {
 		skb_pull(skb, sizeof(*rxd) - 4);
Signature · 1 line
2.20.1
#1 lore raw
Quoted 10 lines · [email protected] writes:

From: Ben Greear <[email protected]>

Print information about whether the message is the first timeout, and also print info if we manage to recover after a timeout.

---

NOTE: Not sure this is really something folks would want upstream, but maybe it helps someone else trying to debug this driver.

If the patch is not ready for upstream it's a good idea to mark it as RFC, that way the maintainers can skip it automatically.

drivers/net/wireless/mediatek/mt76/mt7915/mcu.c mt7915_mcu_parse_response(struct mt76_dev *mdev, int cmd, · line 355

Quoted 12 lines
drivers/net/wireless/mediatek/mt76/mt7915/mcu.c +12 -0
+	if (mdev->first_failed_mcu_cmd) {
+		dev_err(mdev->dev, "MCU: First success after failure: Message %08x (cid %lx ext_cid: %lx seq %d)\n",
+			cmd, FIELD_GET(__MCU_CMD_FIELD_ID, cmd),
+			FIELD_GET(__MCU_CMD_FIELD_EXT_ID, cmd), seq);
+		mdev->first_failed_mcu_cmd = 0;
+	}
+	else {
+		// verbose debugging
+		//dev_err(mdev->dev, "MCU: OK response to message %08x (cid %lx ext_cid: %lx seq %d)\n",
+		//	cmd, FIELD_GET(__MCU_CMD_FIELD_ID, cmd),
+		//	FIELD_GET(__MCU_CMD_FIELD_EXT_ID, cmd), seq);
+	}

No C++ comments and no commented out code, please.

Signature · 3 lines
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
maintainer
#2 lore raw
In reply to [email protected]
Quoted 10 lines · On 2021-10-11 22:54, [email protected] wrote:

From: Ben Greear <[email protected]>

Print information about whether the message is the first timeout, and also print info if we manage to recover after a timeout.

---

NOTE: Not sure this is really something folks would want upstream, but maybe it helps someone else trying to debug this driver.

I think it would be much more useful to add tracepoints for tx/rx mcu messages.

- Felix