Skip to contentSkip to conversation

ath10k: hardware recovery

PATCH Superseded Michal Kazior 2 participants 16 messages Jun 25, 2013 – Jul 23, 2013 1 message not in the archive Qualcomm Atheros ATH10K wireless driver

lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): 8c0a4f68cc3454b77a21ab949075fb0632549bb9

#1 lore raw

v2 cover ath10k: hardware recovery

Respin of hw recovery. I've addressed some issues
raised in the previous review.

I haven't found a way to address the 'scattered
logic' issue (i.e. the way restart_work is
handled).

Michal Kazior (3):
ath10k: implement device recovery
ath10k: implement fw crash simulation command
ath10k: create debugfs interface to trigger fw crash

 drivers/net/wireless/ath/ath10k/core.c  |   30 +++++++++
 drivers/net/wireless/ath/ath10k/core.h  |   16 +++++
 drivers/net/wireless/ath/ath10k/debug.c |   54 +++++++++++++++++
 drivers/net/wireless/ath/ath10k/htc.c   |    3 +
 drivers/net/wireless/ath/ath10k/mac.c   |  101 +++++++++++++++++++++++++------
 drivers/net/wireless/ath/ath10k/mac.h   |    1 +
 drivers/net/wireless/ath/ath10k/pci.c   |    2 +
 drivers/net/wireless/ath/ath10k/wmi.c   |   26 ++++++++
 drivers/net/wireless/ath/ath10k/wmi.h   |   19 ++++++
 9 files changed, 233 insertions(+), 19 deletions(-)
Signature · 1 line
1.7.9.5
#2 lore raw

PATCH v2 1/3 ath10k: implement device recovery

Restart the hardware if FW crashes.

If FW crashes during recovery we leave the hardware in a "wedged" state to avoid recursive recoveries.

When in "wedged" state userspace may bring interfaces down (to issue stop()) and then bring one interface (to issue start()) to reload hardware manually.

---
 drivers/net/wireless/ath/ath10k/core.c |   30 ++++++++++
 drivers/net/wireless/ath/ath10k/core.h |   16 +++++
 drivers/net/wireless/ath/ath10k/htc.c  |    3 +
 drivers/net/wireless/ath/ath10k/mac.c  |  101 ++++++++++++++++++++++++++------
 drivers/net/wireless/ath/ath10k/mac.h  |    1 +
 drivers/net/wireless/ath/ath10k/pci.c  |    2 +
 drivers/net/wireless/ath/ath10k/wmi.c  |    7 +++
 7 files changed, 141 insertions(+), 19 deletions(-)
drivers/net/wireless/ath/ath10k/core.c +30 -0
index c37f79f..503b39f 100644
@@ -476,6 +476,34 @@ static int ath10k_init_hw_params(struct ath10k *ar)
 	return 0;
 }
 
+static void ath10k_core_restart(struct work_struct *work)
+{
+	struct ath10k *ar = container_of(work, struct ath10k, restart_work);
+
+	mutex_lock(&ar->conf_mutex);
+
+	switch (ar->state) {
+	case ATH10K_STATE_ON:
+		ath10k_halt(ar);
+		ar->state = ATH10K_STATE_RESTARTING_OFF;
+		ieee80211_restart_hw(ar->hw);
+		break;
+	case ATH10K_STATE_OFF:
+		/* this can happen if driver is being unloaded */
+		ath10k_warn("cannot restart a device that hasn't been started\n");
+		break;
+	case ATH10K_STATE_RESTARTING_OFF:
+	case ATH10K_STATE_RESTARTING_ON:
+		ar->state = ATH10K_STATE_WEDGED;
+		/* fall through */
+	case ATH10K_STATE_WEDGED:
+		ath10k_warn("device is wedged, will not restart\n");
+		break;
+	}
+
+	mutex_unlock(&ar->conf_mutex);
+}
+
 struct ath10k *ath10k_core_create(void *hif_priv, struct device *dev,
 				  const struct ath10k_hif_ops *hif_ops)
 {
@@ -519,6 +547,8 @@ struct ath10k *ath10k_core_create(void *hif_priv, struct device *dev,
 
 	init_waitqueue_head(&ar->event_queue);
 
+	INIT_WORK(&ar->restart_work, ath10k_core_restart);
+
 	return ar;
 
 err_wq:
drivers/net/wireless/ath/ath10k/core.h +16 -0
index 413f1c5..cf8a3b3 100644
@@ -250,6 +250,20 @@ struct ath10k_debug {
 enum ath10k_state {
 	ATH10K_STATE_OFF = 0,
 	ATH10K_STATE_ON,
+
+	/* When doing firmware recovery the device is first powered down.
+	 * mac80211 is supposed to call in to start() hook later on. It is
+	 * however possible that driver unloading and firmware crash overlap.
+	 * mac80211 can wait on conf_mutex in stop() while the device is
+	 * stopped in ath10k_core_restart() work holding conf_mutex. */
+	ATH10K_STATE_RESTARTING_OFF,
+	ATH10K_STATE_RESTARTING_ON,
+
+	/* The device has crashed while restarting hw. This state is like ON
+	 * but commands are blocked in HTC and -ECOMM response is given. This
+	 * prevents completion timeouts and makes the driver more responsive to
+	 * userspace commands. This is also prevents recursive recovery. */
+	ATH10K_STATE_WEDGED,
 };
 
 struct ath10k {
@@ -355,6 +369,8 @@ struct ath10k {
 
 	enum ath10k_state state;
 
+	struct work_struct restart_work;
+
 #ifdef CONFIG_ATH10K_DEBUGFS
 	struct ath10k_debug debug;
 #endif
drivers/net/wireless/ath/ath10k/htc.c +3 -0
index 7d5a366..72e072c 100644
@@ -246,6 +246,9 @@ int ath10k_htc_send(struct ath10k_htc *htc,
 {
 	struct ath10k_htc_ep *ep = &htc->endpoint[eid];
 
+	if (htc->ar->state == ATH10K_STATE_WEDGED)
+		return -ECOMM;
+
 	if (eid >= ATH10K_HTC_EP_COUNT) {
 		ath10k_warn("Invalid endpoint id: %d\n", eid);
 		return -ENOENT;
drivers/net/wireless/ath/ath10k/mac.c +82 -19

195 lines not on this page · raw message

drivers/net/wireless/ath/ath10k/mac.h +1 -0
index 27fc92e..6fce9bf 100644
@@ -34,6 +34,7 @@ struct ath10k_vif *ath10k_get_arvif(struct ath10k *ar, u32 vdev_id);
 void ath10k_reset_scan(unsigned long ptr);
 void ath10k_offchan_tx_purge(struct ath10k *ar);
 void ath10k_offchan_tx_work(struct work_struct *work);
+void ath10k_halt(struct ath10k *ar);
 
 static inline struct ath10k_vif *ath10k_vif_to_arvif(struct ieee80211_vif *vif)
 {
drivers/net/wireless/ath/ath10k/pci.c +2 -0
index 3790678..e60a927 100644
@@ -720,6 +720,8 @@ static void ath10k_pci_hif_dump_area(struct ath10k *ar)
 			   reg_dump_values[i + 1],
 			   reg_dump_values[i + 2],
 			   reg_dump_values[i + 3]);
+
+	ieee80211_queue_work(ar->hw, &ar->restart_work);
 }
 
 static void ath10k_pci_hif_send_complete_check(struct ath10k *ar, u8 pipe,
drivers/net/wireless/ath/ath10k/wmi.c +7 -0
index b7e7e45..0d25cd7 100644
@@ -27,6 +27,13 @@ void ath10k_wmi_flush_tx(struct ath10k *ar)
 {
 	int ret;
 
+	lockdep_assert_held(&ar->conf_mutex);
+
+	if (ar->state == ATH10K_STATE_WEDGED) {
+		ath10k_warn("wmi flush skipped - device is wedged anyway\n");
+		return;
+	}
+
 	ret = wait_event_timeout(ar->wmi.wq,
 				 atomic_read(&ar->wmi.pending_tx_count) == 0,
 				 5*HZ);
Signature · 1 line
1.7.9.5
#3 lore raw

PATCH v2 1/3 ath10k: implement device recovery

Quoted 12 lines · Michal Kazior

Restart the hardware if FW crashes.

If FW crashes during recovery we leave the hardware in a "wedged" state to avoid recursive recoveries.

When in "wedged" state userspace may bring interfaces down (to issue stop()) and then bring one interface (to issue start()) to reload hardware manually.

[...]

Quoted 4 lines
drivers/net/wireless/ath/ath10k/core.h +0 -0
@@ -250,6 +250,20 @@ struct ath10k_debug {
 enum ath10k_state {

drivers/net/wireless/ath/ath10k/core.h struct ath10k_debug { · line 260

Quoted 10 lines
drivers/net/wireless/ath/ath10k/core.h +8 -0
 	ATH10K_STATE_OFF = 0,
 	ATH10K_STATE_ON,
+
+	/* When doing firmware recovery the device is first powered down.
+	 * mac80211 is supposed to call in to start() hook later on. It is
+	 * however possible that driver unloading and firmware crash overlap.
+	 * mac80211 can wait on conf_mutex in stop() while the device is
+	 * stopped in ath10k_core_restart() work holding conf_mutex. */
+	ATH10K_STATE_RESTARTING_OFF,
+	ATH10K_STATE_RESTARTING_ON,

I feel that RESTARTING_OFF and RESTARTING_ON are a bit confusing names, especially when we have OFF and ON states. Wouldn't RESTARTING and RESTARTED be simpler?

Signature · 1 line
Kalle Valo
#4 lore raw

PATCH v2 1/3 ath10k: implement device recovery

Quoted 27 lines · Kalle Valo
Quoted 12 lines · Michal Kazior

Restart the hardware if FW crashes.

If FW crashes during recovery we leave the hardware in a "wedged" state to avoid recursive recoveries.

When in "wedged" state userspace may bring interfaces down (to issue stop()) and then bring one interface (to issue start()) to reload hardware manually.

[...]

Quoted 10 lines
drivers/net/wireless/ath/ath10k/core.h +4 -0
@@ -250,6 +250,20 @@ struct ath10k_debug {
 enum ath10k_state {
      ATH10K_STATE_OFF = 0,
      ATH10K_STATE_ON,
+
+     /* When doing firmware recovery the device is first powered down.
+      * mac80211 is supposed to call in to start() hook later on. It is
+      * however possible that driver unloading and firmware crash overlap.
Quoted 8 lines

drivers/net/wireless/ath/ath10k/core.h struct ath10k_debug { · line 260

Quoted 4 lines
drivers/net/wireless/ath/ath10k/core.h +4 -0
+      * mac80211 can wait on conf_mutex in stop() while the device is
+      * stopped in ath10k_core_restart() work holding conf_mutex. */
+     ATH10K_STATE_RESTARTING_OFF,
+     ATH10K_STATE_RESTARTING_ON,

I feel that RESTARTING_OFF and RESTARTING_ON are a bit confusing names, especially when we have OFF and ON states. Wouldn't RESTARTING and RESTARTED be simpler?

RESTARTING/RESTARTED seems confusing to me too. The whole thing shouldn't be considered restarted until mac80211 calls in restart_complete() - but then again, at that point we should consider the state as STATE_ON (and we do). Having RESTARTING_OFF / _ON gives off more info about the state hw is in.

Ideally we should have only one state - RESTARTING and guarantee that the hw is in a workable state before we call in for ieee80211_restart_hw(). We'd need to skip hw startup in ath10k_start() in that case and make some code from ath10k_start() reusable and callable from ath10k_core_start().

I can't really remember the reason I went with this particular design now. Perhaps this can be addressed in follow up patches later?

Pozdrawiam / Best regards,
Micha? Kazior.

#5 lore raw

Re: [ath9k-devel] [PATCH v2 1/3] ath10k: implement device recovery

Hi Michal,

sorry, I forgot to answer this one.

Quoted 12 lines · Michal Kazior
Quoted 3 lines · Kalle Valo

I feel that RESTARTING_OFF and RESTARTING_ON are a bit confusing names, especially when we have OFF and ON states. Wouldn't RESTARTING and RESTARTED be simpler?

RESTARTING/RESTARTED seems confusing to me too. The whole thing shouldn't be considered restarted until mac80211 calls in restart_complete() - but then again, at that point we should consider the state as STATE_ON (and we do). Having RESTARTING_OFF / _ON gives off more info about the state hw is in.

Quoted 8 lines

Ideally we should have only one state - RESTARTING and guarantee that the hw is in a workable state before we call in for ieee80211_restart_hw(). We'd need to skip hw startup in ath10k_start() in that case and make some code from ath10k_start() reusable and callable from ath10k_core_start().

I can't really remember the reason I went with this particular design now. Perhaps this can be addressed in follow up patches later?

I would prefer that you still rename the states to what I suggested.

Signature · 1 line
Kalle Valo
#6 lore raw

PATCH v2 2/3 ath10k: implement fw crash simulation command

In reply to Michal Kazior

This can be useful to test FW crash handling.

---
 drivers/net/wireless/ath/ath10k/wmi.c |   19 +++++++++++++++++++
 drivers/net/wireless/ath/ath10k/wmi.h |   19 +++++++++++++++++++
 2 files changed, 38 insertions(+)
drivers/net/wireless/ath/ath10k/wmi.c +19 -0
index 0d25cd7..5e42460 100644
@@ -2092,3 +2092,22 @@ int ath10k_wmi_request_stats(struct ath10k *ar, enum wmi_stats_id stats_id)
 	ath10k_dbg(ATH10K_DBG_WMI, "wmi request stats %d\n", (int)stats_id);
 	return ath10k_wmi_cmd_send(ar, skb, WMI_REQUEST_STATS_CMDID);
 }
+
+int ath10k_wmi_force_fw_hang(struct ath10k *ar,
+			     enum wmi_force_fw_hang_type type, u32 delay_ms)
+{
+	struct wmi_force_fw_hang_cmd *cmd;
+	struct sk_buff *skb;
+
+	skb = ath10k_wmi_alloc_skb(sizeof(*cmd));
+	if (!skb)
+		return -ENOMEM;
+
+	cmd = (struct wmi_force_fw_hang_cmd *)skb->data;
+	cmd->type = __cpu_to_le32(type);
+	cmd->delay_ms = __cpu_to_le32(delay_ms);
+
+	ath10k_dbg(ATH10K_DBG_WMI, "wmi force fw hang %d delay %d\n",
+		   type, delay_ms);
+	return ath10k_wmi_cmd_send(ar, skb, WMI_FORCE_FW_HANG_CMDID);
+}
drivers/net/wireless/ath/ath10k/wmi.h +19 -0
index 9555f5a..da3b2bc 100644
@@ -416,6 +416,7 @@ enum wmi_cmd_id {
 	WMI_PDEV_FTM_INTG_CMDID,
 	WMI_VDEV_SET_KEEPALIVE_CMDID,
 	WMI_VDEV_GET_KEEPALIVE_CMDID,
+	WMI_FORCE_FW_HANG_CMDID,
 
 	/* GPIO Configuration */
 	WMI_GPIO_CONFIG_CMDID = WMI_CMD_GRP(WMI_GRP_GPIO),
@@ -2972,6 +2973,22 @@ struct wmi_sta_keepalive_cmd {
 	struct wmi_sta_keepalive_arp_resp arp_resp;
 } __packed;
 
+enum wmi_force_fw_hang_type {
+	WMI_FORCE_FW_HANG_ASSERT = 1,
+	WMI_FORCE_FW_HANG_NO_DETECT,
+	WMI_FORCE_FW_HANG_CTRL_EP_FULL,
+	WMI_FORCE_FW_HANG_EMPTY_POINT,
+	WMI_FORCE_FW_HANG_STACK_OVERFLOW,
+	WMI_FORCE_FW_HANG_INFINITE_LOOP,
+};
+
+#define WMI_FORCE_FW_HANG_RANDOM_TIME 0xFFFFFFFF
+
+struct wmi_force_fw_hang_cmd {
+	__le32 type;
+	__le32 delay_ms;
+} __packed;
+
 #define ATH10K_RTS_MAX		2347
 #define ATH10K_FRAGMT_THRESHOLD_MIN	540
 #define ATH10K_FRAGMT_THRESHOLD_MAX	2346
@@ -3048,5 +3065,7 @@ int ath10k_wmi_beacon_send(struct ath10k *ar, const struct wmi_bcn_tx_arg *arg);
 int ath10k_wmi_pdev_set_wmm_params(struct ath10k *ar,
 			const struct wmi_pdev_set_wmm_params_arg *arg);
 int ath10k_wmi_request_stats(struct ath10k *ar, enum wmi_stats_id stats_id);
+int ath10k_wmi_force_fw_hang(struct ath10k *ar,
+			     enum wmi_force_fw_hang_type type, u32 delay_ms);
 
 #endif /* _WMI_H_ */
Signature · 1 line
1.7.9.5
#7 lore raw

PATCH v2 3/3 ath10k: create debugfs interface to trigger fw crash

In reply to Michal Kazior

This can be useful for testing.

---
 drivers/net/wireless/ath/ath10k/debug.c |   54 +++++++++++++++++++++++++++++++
 1 file changed, 54 insertions(+)
drivers/net/wireless/ath/ath10k/debug.c +54 -0
index 65279f5..e18f988 100644
@@ -445,6 +445,57 @@ static const struct file_operations fops_fw_stats = {
 	.llseek = default_llseek,
 };
 
+static ssize_t ath10k_read_simulate_fw_crash(struct file *file,
+					     char __user *user_buf,
+					     size_t count, loff_t *ppos)
+{
+	const char buf[] = "To simulate firmware crash write the keyword"
+			   " `crash` to this file.\nThis will force firmware"
+			   " to report a crash to the host system.\n";
+	return simple_read_from_buffer(user_buf, count, ppos, buf, strlen(buf));
+}
+
+static ssize_t ath10k_write_simulate_fw_crash(struct file *file,
+					      const char __user *user_buf,
+					      size_t count, loff_t *ppos)
+{
+	struct ath10k *ar = file->private_data;
+	char buf[32] = {};
+	int ret;
+
+	mutex_lock(&ar->conf_mutex);
+
+	simple_write_to_buffer(buf, sizeof(buf) - 1, ppos, user_buf, count);
+	if (strcmp(buf, "crash") && strcmp(buf, "crash\n")) {
+		ath10k_warn("write keyword `crash` to simulate firmware crash\n");
+		goto exit;
+	}
+
+	if (ar->state != ATH10K_STATE_ON &&
+	    ar->state != ATH10K_STATE_RESTARTING_ON) {
+		ath10k_warn("firmware isn't loaded yet, nothing to crash\n");
+		goto exit;
+	}
+
+	ath10k_info("simulating firmware crash\n");
+
+	ret = ath10k_wmi_force_fw_hang(ar, WMI_FORCE_FW_HANG_ASSERT, 0);
+	if (ret)
+		ath10k_warn("failed to force fw hang (%d)\n", ret);
+
+exit:
+	mutex_unlock(&ar->conf_mutex);
+	return count;
+}
+
+static const struct file_operations fops_simulate_fw_crash = {
+	.read = ath10k_read_simulate_fw_crash,
+	.write = ath10k_write_simulate_fw_crash,
+	.open = simple_open,
+	.owner = THIS_MODULE,
+	.llseek = default_llseek,
+};
+
 int ath10k_debug_create(struct ath10k *ar)
 {
 	ar->debug.debugfs_phy = debugfs_create_dir("ath10k",
@@ -461,6 +512,9 @@ int ath10k_debug_create(struct ath10k *ar)
 	debugfs_create_file("wmi_services", S_IRUSR, ar->debug.debugfs_phy, ar,
 			    &fops_wmi_services);
 
+	debugfs_create_file("simulate_fw_crash", S_IRUSR, ar->debug.debugfs_phy,
+			    ar, &fops_simulate_fw_crash);
+
 	return 0;
 }
 #endif /* CONFIG_ATH10K_DEBUGFS */
Signature · 1 line
1.7.9.5
#8 lore raw

v3 cover ath10k: hardware recovery

In reply to Michal Kazior

Respin of hw recovery. This implements recovery
from a firmware crash.

Note: this is based on my
"ath10k: device setup refactor" patchset.

v3:
* rename RESTARTING_ON/OFF to RESTARTING/RESTARTED
* update commit message for 'create debufs..'

Michal Kazior (3):
ath10k: implement device recovery
ath10k: implement fw crash simulation command
ath10k: create debugfs interface to trigger fw crash

 drivers/net/wireless/ath/ath10k/core.c  |   30 +++++++++
 drivers/net/wireless/ath/ath10k/core.h  |   19 ++++++
 drivers/net/wireless/ath/ath10k/debug.c |   54 +++++++++++++++++
 drivers/net/wireless/ath/ath10k/htc.c   |    3 +
 drivers/net/wireless/ath/ath10k/mac.c   |  101 +++++++++++++++++++++++++------
 drivers/net/wireless/ath/ath10k/mac.h   |    1 +
 drivers/net/wireless/ath/ath10k/pci.c   |    2 +
 drivers/net/wireless/ath/ath10k/wmi.c   |   26 ++++++++
 drivers/net/wireless/ath/ath10k/wmi.h   |   19 ++++++
 9 files changed, 236 insertions(+), 19 deletions(-)
Signature · 1 line
1.7.9.5
#9 lore raw

PATCH v3 1/3 ath10k: implement device recovery

Restart the hardware if FW crashes.

If FW crashes during recovery we leave the hardware in a "wedged" state to avoid recursive recoveries.

When in "wedged" state userspace may bring interfaces down (to issue stop()) and then bring one interface (to issue start()) to reload hardware manually.

---
 drivers/net/wireless/ath/ath10k/core.c |   30 ++++++++++
 drivers/net/wireless/ath/ath10k/core.h |   19 ++++++
 drivers/net/wireless/ath/ath10k/htc.c  |    3 +
 drivers/net/wireless/ath/ath10k/mac.c  |  101 ++++++++++++++++++++++++++------
 drivers/net/wireless/ath/ath10k/mac.h  |    1 +
 drivers/net/wireless/ath/ath10k/pci.c  |    2 +
 drivers/net/wireless/ath/ath10k/wmi.c  |    7 +++
 7 files changed, 144 insertions(+), 19 deletions(-)
drivers/net/wireless/ath/ath10k/core.c +30 -0
index c37f79f..7226c23 100644
@@ -476,6 +476,34 @@ static int ath10k_init_hw_params(struct ath10k *ar)
 	return 0;
 }
 
+static void ath10k_core_restart(struct work_struct *work)
+{
+	struct ath10k *ar = container_of(work, struct ath10k, restart_work);
+
+	mutex_lock(&ar->conf_mutex);
+
+	switch (ar->state) {
+	case ATH10K_STATE_ON:
+		ath10k_halt(ar);
+		ar->state = ATH10K_STATE_RESTARTING;
+		ieee80211_restart_hw(ar->hw);
+		break;
+	case ATH10K_STATE_OFF:
+		/* this can happen if driver is being unloaded */
+		ath10k_warn("cannot restart a device that hasn't been started\n");
+		break;
+	case ATH10K_STATE_RESTARTING:
+	case ATH10K_STATE_RESTARTED:
+		ar->state = ATH10K_STATE_WEDGED;
+		/* fall through */
+	case ATH10K_STATE_WEDGED:
+		ath10k_warn("device is wedged, will not restart\n");
+		break;
+	}
+
+	mutex_unlock(&ar->conf_mutex);
+}
+
 struct ath10k *ath10k_core_create(void *hif_priv, struct device *dev,
 				  const struct ath10k_hif_ops *hif_ops)
 {
@@ -519,6 +547,8 @@ struct ath10k *ath10k_core_create(void *hif_priv, struct device *dev,
 
 	init_waitqueue_head(&ar->event_queue);
 
+	INIT_WORK(&ar->restart_work, ath10k_core_restart);
+
 	return ar;
 
 err_wq:
drivers/net/wireless/ath/ath10k/core.h +19 -0
index 413f1c5..9f21ecb 100644
@@ -250,6 +250,23 @@ struct ath10k_debug {
 enum ath10k_state {
 	ATH10K_STATE_OFF = 0,
 	ATH10K_STATE_ON,
+
+	/* When doing firmware recovery the device is first powered down.
+	 * mac80211 is supposed to call in to start() hook later on. It is
+	 * however possible that driver unloading and firmware crash overlap.
+	 * mac80211 can wait on conf_mutex in stop() while the device is
+	 * stopped in ath10k_core_restart() work holding conf_mutex. The state
+	 * RESTARTED means that the device is up and mac80211 has started hw
+	 * reconfiguration. Once mac80211 is done with the reconfiguration we
+	 * set the state to STATE_ON in restart_complete(). */
+	ATH10K_STATE_RESTARTING,
+	ATH10K_STATE_RESTARTED,
+
+	/* The device has crashed while restarting hw. This state is like ON
+	 * but commands are blocked in HTC and -ECOMM response is given. This
+	 * prevents completion timeouts and makes the driver more responsive to
+	 * userspace commands. This is also prevents recursive recovery. */
+	ATH10K_STATE_WEDGED,
 };
 
 struct ath10k {
@@ -355,6 +372,8 @@ struct ath10k {
 
 	enum ath10k_state state;
 
+	struct work_struct restart_work;
+
 #ifdef CONFIG_ATH10K_DEBUGFS
 	struct ath10k_debug debug;
 #endif
drivers/net/wireless/ath/ath10k/htc.c +3 -0
index 7d5a366..72e072c 100644
@@ -246,6 +246,9 @@ int ath10k_htc_send(struct ath10k_htc *htc,
 {
 	struct ath10k_htc_ep *ep = &htc->endpoint[eid];
 
+	if (htc->ar->state == ATH10K_STATE_WEDGED)
+		return -ECOMM;
+
 	if (eid >= ATH10K_HTC_EP_COUNT) {
 		ath10k_warn("Invalid endpoint id: %d\n", eid);
 		return -ENOENT;
drivers/net/wireless/ath/ath10k/mac.c +82 -19

195 lines not on this page · raw message

drivers/net/wireless/ath/ath10k/mac.h +1 -0
index 27fc92e..6fce9bf 100644
@@ -34,6 +34,7 @@ struct ath10k_vif *ath10k_get_arvif(struct ath10k *ar, u32 vdev_id);
 void ath10k_reset_scan(unsigned long ptr);
 void ath10k_offchan_tx_purge(struct ath10k *ar);
 void ath10k_offchan_tx_work(struct work_struct *work);
+void ath10k_halt(struct ath10k *ar);
 
 static inline struct ath10k_vif *ath10k_vif_to_arvif(struct ieee80211_vif *vif)
 {
drivers/net/wireless/ath/ath10k/pci.c +2 -0
index bfe8561..c71b488 100644
@@ -720,6 +720,8 @@ static void ath10k_pci_hif_dump_area(struct ath10k *ar)
 			   reg_dump_values[i + 1],
 			   reg_dump_values[i + 2],
 			   reg_dump_values[i + 3]);
+
+	ieee80211_queue_work(ar->hw, &ar->restart_work);
 }
 
 static void ath10k_pci_hif_send_complete_check(struct ath10k *ar, u8 pipe,
drivers/net/wireless/ath/ath10k/wmi.c +7 -0
index b7e7e45..0d25cd7 100644
@@ -27,6 +27,13 @@ void ath10k_wmi_flush_tx(struct ath10k *ar)
 {
 	int ret;
 
+	lockdep_assert_held(&ar->conf_mutex);
+
+	if (ar->state == ATH10K_STATE_WEDGED) {
+		ath10k_warn("wmi flush skipped - device is wedged anyway\n");
+		return;
+	}
+
 	ret = wait_event_timeout(ar->wmi.wq,
 				 atomic_read(&ar->wmi.pending_tx_count) == 0,
 				 5*HZ);
Signature · 1 line
1.7.9.5
#10 lore raw

PATCH v3 2/3 ath10k: implement fw crash simulation command

In reply to Michal Kazior

This can be useful to test FW crash handling.

---
 drivers/net/wireless/ath/ath10k/wmi.c |   19 +++++++++++++++++++
 drivers/net/wireless/ath/ath10k/wmi.h |   19 +++++++++++++++++++
 2 files changed, 38 insertions(+)
drivers/net/wireless/ath/ath10k/wmi.c +19 -0
index 0d25cd7..5e42460 100644
@@ -2092,3 +2092,22 @@ int ath10k_wmi_request_stats(struct ath10k *ar, enum wmi_stats_id stats_id)
 	ath10k_dbg(ATH10K_DBG_WMI, "wmi request stats %d\n", (int)stats_id);
 	return ath10k_wmi_cmd_send(ar, skb, WMI_REQUEST_STATS_CMDID);
 }
+
+int ath10k_wmi_force_fw_hang(struct ath10k *ar,
+			     enum wmi_force_fw_hang_type type, u32 delay_ms)
+{
+	struct wmi_force_fw_hang_cmd *cmd;
+	struct sk_buff *skb;
+
+	skb = ath10k_wmi_alloc_skb(sizeof(*cmd));
+	if (!skb)
+		return -ENOMEM;
+
+	cmd = (struct wmi_force_fw_hang_cmd *)skb->data;
+	cmd->type = __cpu_to_le32(type);
+	cmd->delay_ms = __cpu_to_le32(delay_ms);
+
+	ath10k_dbg(ATH10K_DBG_WMI, "wmi force fw hang %d delay %d\n",
+		   type, delay_ms);
+	return ath10k_wmi_cmd_send(ar, skb, WMI_FORCE_FW_HANG_CMDID);
+}
drivers/net/wireless/ath/ath10k/wmi.h +19 -0
index 9555f5a..da3b2bc 100644
@@ -416,6 +416,7 @@ enum wmi_cmd_id {
 	WMI_PDEV_FTM_INTG_CMDID,
 	WMI_VDEV_SET_KEEPALIVE_CMDID,
 	WMI_VDEV_GET_KEEPALIVE_CMDID,
+	WMI_FORCE_FW_HANG_CMDID,
 
 	/* GPIO Configuration */
 	WMI_GPIO_CONFIG_CMDID = WMI_CMD_GRP(WMI_GRP_GPIO),
@@ -2972,6 +2973,22 @@ struct wmi_sta_keepalive_cmd {
 	struct wmi_sta_keepalive_arp_resp arp_resp;
 } __packed;
 
+enum wmi_force_fw_hang_type {
+	WMI_FORCE_FW_HANG_ASSERT = 1,
+	WMI_FORCE_FW_HANG_NO_DETECT,
+	WMI_FORCE_FW_HANG_CTRL_EP_FULL,
+	WMI_FORCE_FW_HANG_EMPTY_POINT,
+	WMI_FORCE_FW_HANG_STACK_OVERFLOW,
+	WMI_FORCE_FW_HANG_INFINITE_LOOP,
+};
+
+#define WMI_FORCE_FW_HANG_RANDOM_TIME 0xFFFFFFFF
+
+struct wmi_force_fw_hang_cmd {
+	__le32 type;
+	__le32 delay_ms;
+} __packed;
+
 #define ATH10K_RTS_MAX		2347
 #define ATH10K_FRAGMT_THRESHOLD_MIN	540
 #define ATH10K_FRAGMT_THRESHOLD_MAX	2346
@@ -3048,5 +3065,7 @@ int ath10k_wmi_beacon_send(struct ath10k *ar, const struct wmi_bcn_tx_arg *arg);
 int ath10k_wmi_pdev_set_wmm_params(struct ath10k *ar,
 			const struct wmi_pdev_set_wmm_params_arg *arg);
 int ath10k_wmi_request_stats(struct ath10k *ar, enum wmi_stats_id stats_id);
+int ath10k_wmi_force_fw_hang(struct ath10k *ar,
+			     enum wmi_force_fw_hang_type type, u32 delay_ms);
 
 #endif /* _WMI_H_ */
Signature · 1 line
1.7.9.5
#11 lore raw

PATCH v3 3/3 ath10k: create debugfs interface to trigger fw crash

In reply to Michal Kazior

This can be useful for testing. To perform a
forced firmware crash write 'crash' to
'simulate_fw_crash' debugfs file. E.g.

echo crash > /sys/kernel/debug/ieee80211/phy1/ath10k/simulate_fw_crash

---
 drivers/net/wireless/ath/ath10k/debug.c |   54 +++++++++++++++++++++++++++++++
 1 file changed, 54 insertions(+)
drivers/net/wireless/ath/ath10k/debug.c +54 -0
index 65279f5..4021bea 100644
@@ -445,6 +445,57 @@ static const struct file_operations fops_fw_stats = {
 	.llseek = default_llseek,
 };
 
+static ssize_t ath10k_read_simulate_fw_crash(struct file *file,
+					     char __user *user_buf,
+					     size_t count, loff_t *ppos)
+{
+	const char buf[] = "To simulate firmware crash write the keyword"
+			   " `crash` to this file.\nThis will force firmware"
+			   " to report a crash to the host system.\n";
+	return simple_read_from_buffer(user_buf, count, ppos, buf, strlen(buf));
+}
+
+static ssize_t ath10k_write_simulate_fw_crash(struct file *file,
+					      const char __user *user_buf,
+					      size_t count, loff_t *ppos)
+{
+	struct ath10k *ar = file->private_data;
+	char buf[32] = {};
+	int ret;
+
+	mutex_lock(&ar->conf_mutex);
+
+	simple_write_to_buffer(buf, sizeof(buf) - 1, ppos, user_buf, count);
+	if (strcmp(buf, "crash") && strcmp(buf, "crash\n")) {
+		ath10k_warn("write keyword `crash` to simulate firmware crash\n");
+		goto exit;
+	}
+
+	if (ar->state != ATH10K_STATE_ON &&
+	    ar->state != ATH10K_STATE_RESTARTED) {
+		ath10k_warn("firmware isn't loaded yet, nothing to crash\n");
+		goto exit;
+	}
+
+	ath10k_info("simulating firmware crash\n");
+
+	ret = ath10k_wmi_force_fw_hang(ar, WMI_FORCE_FW_HANG_ASSERT, 0);
+	if (ret)
+		ath10k_warn("failed to force fw hang (%d)\n", ret);
+
+exit:
+	mutex_unlock(&ar->conf_mutex);
+	return count;
+}
+
+static const struct file_operations fops_simulate_fw_crash = {
+	.read = ath10k_read_simulate_fw_crash,
+	.write = ath10k_write_simulate_fw_crash,
+	.open = simple_open,
+	.owner = THIS_MODULE,
+	.llseek = default_llseek,
+};
+
 int ath10k_debug_create(struct ath10k *ar)
 {
 	ar->debug.debugfs_phy = debugfs_create_dir("ath10k",
@@ -461,6 +512,9 @@ int ath10k_debug_create(struct ath10k *ar)
 	debugfs_create_file("wmi_services", S_IRUSR, ar->debug.debugfs_phy, ar,
 			    &fops_wmi_services);
 
+	debugfs_create_file("simulate_fw_crash", S_IRUSR, ar->debug.debugfs_phy,
+			    ar, &fops_simulate_fw_crash);
+
 	return 0;
 }
 #endif /* CONFIG_ATH10K_DEBUGFS */
Signature · 1 line
1.7.9.5
#12 lore raw

Re: [PATCH v3 3/3] ath10k: create debugfs interface to trigger fw crash

Quoted 7 lines · Michal Kazior

This can be useful for testing. To perform a
forced firmware crash write 'crash' to
'simulate_fw_crash' debugfs file. E.g.

echo crash > /sys/kernel/debug/ieee80211/phy1/ath10k/simulate_fw_crash

[...]

Quoted 5 lines
drivers/net/wireless/ath/ath10k/debug.c +5 -0
+static ssize_t ath10k_write_simulate_fw_crash(struct file *file,
+					      const char __user *user_buf,
+					      size_t count, loff_t *ppos)
+{
+	struct ath10k *ar = file->private_data;

drivers/net/wireless/ath/ath10k/debug.c static const struct file_operations fops_fw_stats = { · line 472

Quoted 10 lines
drivers/net/wireless/ath/ath10k/debug.c +10 -0
+	char buf[32] = {};
+	int ret;
+
+	mutex_lock(&ar->conf_mutex);
+
+	simple_write_to_buffer(buf, sizeof(buf) - 1, ppos, user_buf, count);
+	if (strcmp(buf, "crash") && strcmp(buf, "crash\n")) {
+		ath10k_warn("write keyword `crash` to simulate firmware crash\n");
+		goto exit;
+	}

Better to just return an error here.

drivers/net/wireless/ath/ath10k/debug.c static const struct file_operations fops_fw_stats = { · line 478

Quoted 5 lines
drivers/net/wireless/ath/ath10k/debug.c +5 -0
+	if (ar->state != ATH10K_STATE_ON &&
+	    ar->state != ATH10K_STATE_RESTARTED) {
+		ath10k_warn("firmware isn't loaded yet, nothing to crash\n");
+		goto exit;
+	}

Ditto.

Signature · 1 line
Kalle Valo
#13 lore raw

Re: [PATCH v3 3/3] ath10k: create debugfs interface to trigger fw crash

Quoted 18 lines · Kalle Valo
Quoted 5 lines
drivers/net/wireless/ath/ath10k/debug.c +5 -0
+static ssize_t ath10k_write_simulate_fw_crash(struct file *file,
+					      const char __user *user_buf,
+					      size_t count, loff_t *ppos)
+{
+	struct ath10k *ar = file->private_data;

drivers/net/wireless/ath/ath10k/debug.c static const struct file_operations fops_fw_stats = { · line 472

Quoted 10 lines
drivers/net/wireless/ath/ath10k/debug.c +10 -0
+	char buf[32] = {};
+	int ret;
+
+	mutex_lock(&ar->conf_mutex);
+
+	simple_write_to_buffer(buf, sizeof(buf) - 1, ppos, user_buf, count);
+	if (strcmp(buf, "crash") && strcmp(buf, "crash\n")) {
+		ath10k_warn("write keyword `crash` to simulate firmware crash\n");
+		goto exit;
+	}

Better to just return an error here.

Quoted 7 lines

drivers/net/wireless/ath/ath10k/debug.c static const struct file_operations fops_fw_stats = { · line 478

Quoted 5 lines
drivers/net/wireless/ath/ath10k/debug.c +5 -0
+	if (ar->state != ATH10K_STATE_ON &&
+	    ar->state != ATH10K_STATE_RESTARTED) {
+		ath10k_warn("firmware isn't loaded yet, nothing to crash\n");
+		goto exit;
+	}

Ditto.

Sorry, I was unclear here. I also meant that the two ath10k_warn() calls should be removed.

Signature · 1 line
Kalle Valo
#14 lore raw

PATCH v4 1/1 ath10k: create debugfs interface to trigger fw crash

This can be useful for testing. To perform a
forced firmware crash write 'crash' to
'simulate_fw_crash' debugfs file. E.g.

echo crash > /sys/kernel/debug/ieee80211/phy1/ath10k/simulate_fw_crash

---
v3: return an appropriate errno instead of using ath10k_warn()

 drivers/net/wireless/ath/ath10k/debug.c |   57 +++++++++++++++++++++++++++++++
 1 file changed, 57 insertions(+)
drivers/net/wireless/ath/ath10k/debug.c +57 -0
index 65279f5..3d65594 100644
@@ -445,6 +445,60 @@ static const struct file_operations fops_fw_stats = {
 	.llseek = default_llseek,
 };
 
+static ssize_t ath10k_read_simulate_fw_crash(struct file *file,
+					     char __user *user_buf,
+					     size_t count, loff_t *ppos)
+{
+	const char buf[] = "To simulate firmware crash write the keyword"
+			   " `crash` to this file.\nThis will force firmware"
+			   " to report a crash to the host system.\n";
+	return simple_read_from_buffer(user_buf, count, ppos, buf, strlen(buf));
+}
+
+static ssize_t ath10k_write_simulate_fw_crash(struct file *file,
+					      const char __user *user_buf,
+					      size_t count, loff_t *ppos)
+{
+	struct ath10k *ar = file->private_data;
+	char buf[32] = {};
+	int ret;
+
+	mutex_lock(&ar->conf_mutex);
+
+	simple_write_to_buffer(buf, sizeof(buf) - 1, ppos, user_buf, count);
+	if (strcmp(buf, "crash") && strcmp(buf, "crash\n")) {
+		ret = -EINVAL;
+		goto exit;
+	}
+
+	if (ar->state != ATH10K_STATE_ON &&
+	    ar->state != ATH10K_STATE_RESTARTED) {
+		ret = -ENETDOWN;
+		goto exit;
+	}
+
+	ath10k_info("simulating firmware crash\n");
+
+	ret = ath10k_wmi_force_fw_hang(ar, WMI_FORCE_FW_HANG_ASSERT, 0);
+	if (ret)
+		ath10k_warn("failed to force fw hang (%d)\n", ret);
+
+	if (ret == 0)
+		ret = count;
+
+exit:
+	mutex_unlock(&ar->conf_mutex);
+	return ret;
+}
+
+static const struct file_operations fops_simulate_fw_crash = {
+	.read = ath10k_read_simulate_fw_crash,
+	.write = ath10k_write_simulate_fw_crash,
+	.open = simple_open,
+	.owner = THIS_MODULE,
+	.llseek = default_llseek,
+};
+
 int ath10k_debug_create(struct ath10k *ar)
 {
 	ar->debug.debugfs_phy = debugfs_create_dir("ath10k",
@@ -461,6 +515,9 @@ int ath10k_debug_create(struct ath10k *ar)
 	debugfs_create_file("wmi_services", S_IRUSR, ar->debug.debugfs_phy, ar,
 			    &fops_wmi_services);
 
+	debugfs_create_file("simulate_fw_crash", S_IRUSR, ar->debug.debugfs_phy,
+			    ar, &fops_simulate_fw_crash);
+
 	return 0;
 }
 #endif /* CONFIG_ATH10K_DEBUGFS */
Signature · 1 line
1.7.9.5
Applied
#15 lore raw

Re: [PATCH v4] ath10k: create debugfs interface to trigger fw crash

Quoted 9 lines · Michal Kazior

This can be useful for testing. To perform a
forced firmware crash write 'crash' to
'simulate_fw_crash' debugfs file. E.g.

echo crash > /sys/kernel/debug/ieee80211/phy1/ath10k/simulate_fw_crash

---
v3: return an appropriate errno instead of using ath10k_warn()

Thanks, applied.

Signature · 1 line
Kalle Valo
Applied
#16 lore raw
In reply to Michal Kazior
Quoted 6 lines · Michal Kazior

Respin of hw recovery. This implements recovery
from a firmware crash.

Note: this is based on my
"ath10k: device setup refactor" patchset.

Quoted 8 lines

v3:
* rename RESTARTING_ON/OFF to RESTARTING/RESTARTED
* update commit message for 'create debufs..'

Michal Kazior (3):
ath10k: implement device recovery
ath10k: implement fw crash simulation command
ath10k: create debugfs interface to trigger fw crash

Thanks, applied patches 1-2 and dropped patch 3.

Signature · 1 line
Kalle Valo