All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] ath10k:  fix vdev-start timeout on error
@ 2018-07-13 20:09 ` greearb
  0 siblings, 0 replies; 6+ messages in thread
From: greearb @ 2018-07-13 20:09 UTC (permalink / raw)
  To: linux-wireless, ath10k, kvalo; +Cc: Ben Greear

From: Ben Greear <greearb@candelatech.com>

The vdev-start-response message should cause the
completion to fire, even in the error case.  Otherwise,
the user still gets no useful information and everything
is blocked until the timeout period.

Add some warning text to print out the invalid status
code to aid debugging, and propagate failure code.

Signed-off-by: Ben Greear <greearb@candelatech.com>
---

v2:  Propagate failure code

 drivers/net/wireless/ath/ath10k/core.h |  1 +
 drivers/net/wireless/ath/ath10k/mac.c  |  2 +-
 drivers/net/wireless/ath/ath10k/wmi.c  | 19 ++++++++++++++++---
 drivers/net/wireless/ath/ath10k/wmi.h  |  8 +++++++-
 4 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index e3ec4d4..df232a4 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -1112,6 +1112,7 @@ struct ath10k {
 
 	struct completion install_key_done;
 
+	int last_wmi_vdev_start_status;
 	struct completion vdev_setup_done;
 
 	struct workqueue_struct *workqueue;
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 85b8398..c90c8bb 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -1054,7 +1054,7 @@ static inline int ath10k_vdev_setup_sync(struct ath10k *ar)
 	if (time_left == 0)
 		return -ETIMEDOUT;
 
-	return 0;
+	return ar->last_wmi_vdev_start_status;
 }
 
 static int ath10k_monitor_vdev_start(struct ath10k *ar, int vdev_id)
diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
index a60de71..6aaa439 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -3437,18 +3437,31 @@ void ath10k_wmi_event_vdev_start_resp(struct ath10k *ar, struct sk_buff *skb)
 {
 	struct wmi_vdev_start_ev_arg arg = {};
 	int ret;
+	u32 status;
 
 	ath10k_dbg(ar, ATH10K_DBG_WMI, "WMI_VDEV_START_RESP_EVENTID\n");
 
+	ar->last_wmi_vdev_start_status = 0;
+
 	ret = ath10k_wmi_pull_vdev_start(ar, skb, &arg);
 	if (ret) {
 		ath10k_warn(ar, "failed to parse vdev start event: %d\n", ret);
-		return;
+		ar->last_wmi_vdev_start_status = ret;
+		goto out;
 	}
 
-	if (WARN_ON(__le32_to_cpu(arg.status)))
-		return;
+	status = __le32_to_cpu(arg.status);
+	if (WARN_ON_ONCE(status)) {
+		ath10k_warn(ar, "vdev-start-response reports status error: %d (%s)\n",
+			    status, (status == WMI_VDEV_START_CHAN_INVALID) ?
+			    "chan-invalid" : "unknown");
+		/* Setup is done one way or another though, so we should still
+		 * do the completion, so don't return here.
+		 */
+		ar->last_wmi_vdev_start_status = -EINVAL;
+	}
 
+out:
 	complete(&ar->vdev_setup_done);
 }
 
diff --git a/drivers/net/wireless/ath/ath10k/wmi.h b/drivers/net/wireless/ath/ath10k/wmi.h
index 9ba9625..95ff280 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.h
+++ b/drivers/net/wireless/ath/ath10k/wmi.h
@@ -6778,11 +6778,17 @@ struct wmi_ch_info_ev_arg {
 	__le32 rx_frame_count;
 };
 
+/* From 10.4 firmware, not sure all have the same values. */
+enum wmi_vdev_start_status {
+        WMI_VDEV_START_OK = 0,
+        WMI_VDEV_START_CHAN_INVALID,
+};
+
 struct wmi_vdev_start_ev_arg {
 	__le32 vdev_id;
 	__le32 req_id;
 	__le32 resp_type; /* %WMI_VDEV_RESP_ */
-	__le32 status;
+	__le32 status; /* See wmi_vdev_start_status enum above */
 };
 
 struct wmi_peer_kick_ev_arg {
-- 
2.4.11

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v2] ath10k:  fix vdev-start timeout on error
@ 2018-07-13 20:09 ` greearb
  0 siblings, 0 replies; 6+ messages in thread
From: greearb @ 2018-07-13 20:09 UTC (permalink / raw)
  To: linux-wireless, ath10k, kvalo; +Cc: Ben Greear

From: Ben Greear <greearb@candelatech.com>

The vdev-start-response message should cause the
completion to fire, even in the error case.  Otherwise,
the user still gets no useful information and everything
is blocked until the timeout period.

Add some warning text to print out the invalid status
code to aid debugging, and propagate failure code.

Signed-off-by: Ben Greear <greearb@candelatech.com>
---

v2:  Propagate failure code

 drivers/net/wireless/ath/ath10k/core.h |  1 +
 drivers/net/wireless/ath/ath10k/mac.c  |  2 +-
 drivers/net/wireless/ath/ath10k/wmi.c  | 19 ++++++++++++++++---
 drivers/net/wireless/ath/ath10k/wmi.h  |  8 +++++++-
 4 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index e3ec4d4..df232a4 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -1112,6 +1112,7 @@ struct ath10k {
 
 	struct completion install_key_done;
 
+	int last_wmi_vdev_start_status;
 	struct completion vdev_setup_done;
 
 	struct workqueue_struct *workqueue;
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 85b8398..c90c8bb 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -1054,7 +1054,7 @@ static inline int ath10k_vdev_setup_sync(struct ath10k *ar)
 	if (time_left == 0)
 		return -ETIMEDOUT;
 
-	return 0;
+	return ar->last_wmi_vdev_start_status;
 }
 
 static int ath10k_monitor_vdev_start(struct ath10k *ar, int vdev_id)
diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
index a60de71..6aaa439 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -3437,18 +3437,31 @@ void ath10k_wmi_event_vdev_start_resp(struct ath10k *ar, struct sk_buff *skb)
 {
 	struct wmi_vdev_start_ev_arg arg = {};
 	int ret;
+	u32 status;
 
 	ath10k_dbg(ar, ATH10K_DBG_WMI, "WMI_VDEV_START_RESP_EVENTID\n");
 
+	ar->last_wmi_vdev_start_status = 0;
+
 	ret = ath10k_wmi_pull_vdev_start(ar, skb, &arg);
 	if (ret) {
 		ath10k_warn(ar, "failed to parse vdev start event: %d\n", ret);
-		return;
+		ar->last_wmi_vdev_start_status = ret;
+		goto out;
 	}
 
-	if (WARN_ON(__le32_to_cpu(arg.status)))
-		return;
+	status = __le32_to_cpu(arg.status);
+	if (WARN_ON_ONCE(status)) {
+		ath10k_warn(ar, "vdev-start-response reports status error: %d (%s)\n",
+			    status, (status == WMI_VDEV_START_CHAN_INVALID) ?
+			    "chan-invalid" : "unknown");
+		/* Setup is done one way or another though, so we should still
+		 * do the completion, so don't return here.
+		 */
+		ar->last_wmi_vdev_start_status = -EINVAL;
+	}
 
+out:
 	complete(&ar->vdev_setup_done);
 }
 
diff --git a/drivers/net/wireless/ath/ath10k/wmi.h b/drivers/net/wireless/ath/ath10k/wmi.h
index 9ba9625..95ff280 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.h
+++ b/drivers/net/wireless/ath/ath10k/wmi.h
@@ -6778,11 +6778,17 @@ struct wmi_ch_info_ev_arg {
 	__le32 rx_frame_count;
 };
 
+/* From 10.4 firmware, not sure all have the same values. */
+enum wmi_vdev_start_status {
+        WMI_VDEV_START_OK = 0,
+        WMI_VDEV_START_CHAN_INVALID,
+};
+
 struct wmi_vdev_start_ev_arg {
 	__le32 vdev_id;
 	__le32 req_id;
 	__le32 resp_type; /* %WMI_VDEV_RESP_ */
-	__le32 status;
+	__le32 status; /* See wmi_vdev_start_status enum above */
 };
 
 struct wmi_peer_kick_ev_arg {
-- 
2.4.11


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] ath10k:  fix vdev-start timeout on error
  2018-07-13 20:09 ` greearb
  (?)
  (?)
@ 2018-09-04  5:14 ` Kalle Valo
  -1 siblings, 0 replies; 6+ messages in thread
From: Kalle Valo @ 2018-09-04  5:14 UTC (permalink / raw)
  To: greearb; +Cc: linux-wireless, ath10k, Ben Greear

greearb@candelatech.com wrote:

> The vdev-start-response message should cause the
> completion to fire, even in the error case.  Otherwise,
> the user still gets no useful information and everything
> is blocked until the timeout period.
> 
> Add some warning text to print out the invalid status
> code to aid debugging, and propagate failure code.
> 
> Signed-off-by: Ben Greear <greearb@candelatech.com>
> Signed-off-by: Kalle Valo <kvalo@codeaurora.org>

This had few checkpatch warnings:

drivers/net/wireless/ath/ath10k/wmi.h:6647: code indent should use tabs where possible
drivers/net/wireless/ath/ath10k/wmi.h:6647: please, no spaces at the start of a line
drivers/net/wireless/ath/ath10k/wmi.h:6648: code indent should use tabs where possible
drivers/net/wireless/ath/ath10k/wmi.h:6648: please, no spaces at the start of a line

I fixed those in the pending branch.

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

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] ath10k:  fix vdev-start timeout on error
  2018-07-13 20:09 ` greearb
  (?)
@ 2018-09-04  5:14 ` Kalle Valo
  -1 siblings, 0 replies; 6+ messages in thread
From: Kalle Valo @ 2018-09-04  5:14 UTC (permalink / raw)
  To: greearb; +Cc: linux-wireless, ath10k

greearb@candelatech.com wrote:

> The vdev-start-response message should cause the
> completion to fire, even in the error case.  Otherwise,
> the user still gets no useful information and everything
> is blocked until the timeout period.
> 
> Add some warning text to print out the invalid status
> code to aid debugging, and propagate failure code.
> 
> Signed-off-by: Ben Greear <greearb@candelatech.com>
> Signed-off-by: Kalle Valo <kvalo@codeaurora.org>

This had few checkpatch warnings:

drivers/net/wireless/ath/ath10k/wmi.h:6647: code indent should use tabs where possible
drivers/net/wireless/ath/ath10k/wmi.h:6647: please, no spaces at the start of a line
drivers/net/wireless/ath/ath10k/wmi.h:6648: code indent should use tabs where possible
drivers/net/wireless/ath/ath10k/wmi.h:6648: please, no spaces at the start of a line

I fixed those in the pending branch.

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

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


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] ath10k:  fix vdev-start timeout on error
  2018-07-13 20:09 ` greearb
                   ` (3 preceding siblings ...)
  (?)
@ 2018-10-01 13:33 ` Kalle Valo
  -1 siblings, 0 replies; 6+ messages in thread
From: Kalle Valo @ 2018-10-01 13:33 UTC (permalink / raw)
  To: greearb; +Cc: linux-wireless, ath10k, Ben Greear

greearb@candelatech.com wrote:

> The vdev-start-response message should cause the
> completion to fire, even in the error case.  Otherwise,
> the user still gets no useful information and everything
> is blocked until the timeout period.
> 
> Add some warning text to print out the invalid status
> code to aid debugging, and propagate failure code.
> 
> Signed-off-by: Ben Greear <greearb@candelatech.com>
> Signed-off-by: Kalle Valo <kvalo@codeaurora.org>

Patch applied to ath-next branch of ath.git, thanks.

833fd34d743c ath10k: fix vdev-start timeout on error

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

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] ath10k:  fix vdev-start timeout on error
  2018-07-13 20:09 ` greearb
                   ` (2 preceding siblings ...)
  (?)
@ 2018-10-01 13:33 ` Kalle Valo
  -1 siblings, 0 replies; 6+ messages in thread
From: Kalle Valo @ 2018-10-01 13:33 UTC (permalink / raw)
  To: greearb; +Cc: linux-wireless, ath10k

greearb@candelatech.com wrote:

> The vdev-start-response message should cause the
> completion to fire, even in the error case.  Otherwise,
> the user still gets no useful information and everything
> is blocked until the timeout period.
> 
> Add some warning text to print out the invalid status
> code to aid debugging, and propagate failure code.
> 
> Signed-off-by: Ben Greear <greearb@candelatech.com>
> Signed-off-by: Kalle Valo <kvalo@codeaurora.org>

Patch applied to ath-next branch of ath.git, thanks.

833fd34d743c ath10k: fix vdev-start timeout on error

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

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


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2018-10-01 13:33 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-13 20:09 [PATCH v2] ath10k: fix vdev-start timeout on error greearb
2018-07-13 20:09 ` greearb
2018-09-04  5:14 ` Kalle Valo
2018-09-04  5:14 ` Kalle Valo
2018-10-01 13:33 ` Kalle Valo
2018-10-01 13:33 ` Kalle Valo

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.