[PATCH 0/4] ath10k: fix spurious tx/rx during boot

[PATCH 0/4] ath10k: fix spurious tx/rx during boot

[Michal Kazior]

Hi,

Recently Marek discovered the device transmits
"stuff" during device/driver boot.

The problem is related with the long known "no
channel" warning and it's a consequence of the
same bug - rx filters not being programmed
properly by firmware.

See last patch for more details.

I didn't do extensive testing but I can confirm
that I am no longer able to reroduce "no channel"
warnings and Marek tells me he no longer sees any
signal bumps on oscilloscope with his QCA9882.



Michal Kazior (4):
  ath10k: implement wmi echo command
  ath10k: implement wmi echo event
  ath10k: add wmi command barrier utility
  ath10k: fix spurious tx/rx during boot

 drivers/net/wireless/ath/ath10k/core.c    | 68 +++++++++++++++++++++++++
 drivers/net/wireless/ath/ath10k/core.h    |  1 +
 drivers/net/wireless/ath/ath10k/wmi-ops.h | 29 +++++++++++
 drivers/net/wireless/ath/ath10k/wmi-tlv.c | 57 +++++++++++++++++++++
 drivers/net/wireless/ath/ath10k/wmi.c     | 83 ++++++++++++++++++++++++++++++-
 drivers/net/wireless/ath/ath10k/wmi.h     |  5 ++
 6 files changed, 242 insertions(+), 1 deletion(-)

-- 
2.1.4

[PATCH 0/4] ath10k: fix spurious tx/rx during boot

[Michal Kazior]

Hi,

Recently Marek discovered the device transmits
"stuff" during device/driver boot.

The problem is related with the long known "no
channel" warning and it's a consequence of the
same bug - rx filters not being programmed
properly by firmware.

See last patch for more details.

I didn't do extensive testing but I can confirm
that I am no longer able to reroduce "no channel"
warnings and Marek tells me he no longer sees any
signal bumps on oscilloscope with his QCA9882.



Michal Kazior (4):
  ath10k: implement wmi echo command
  ath10k: implement wmi echo event
  ath10k: add wmi command barrier utility
  ath10k: fix spurious tx/rx during boot

 drivers/net/wireless/ath/ath10k/core.c    | 68 +++++++++++++++++++++++++
 drivers/net/wireless/ath/ath10k/core.h    |  1 +
 drivers/net/wireless/ath/ath10k/wmi-ops.h | 29 +++++++++++
 drivers/net/wireless/ath/ath10k/wmi-tlv.c | 57 +++++++++++++++++++++
 drivers/net/wireless/ath/ath10k/wmi.c     | 83 ++++++++++++++++++++++++++++++-
 drivers/net/wireless/ath/ath10k/wmi.h     |  5 ++
 6 files changed, 242 insertions(+), 1 deletion(-)

-- 
2.1.4


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

[PATCH 1/4] ath10k: implement wmi echo command

[Michal Kazior]

Will be useful for implementing command barriers.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/wmi-ops.h | 17 +++++++++++++++++
 drivers/net/wireless/ath/ath10k/wmi-tlv.c | 29 +++++++++++++++++++++++++++++
 drivers/net/wireless/ath/ath10k/wmi.c     | 23 +++++++++++++++++++++++
 3 files changed, 69 insertions(+)

diff --git a/drivers/net/wireless/ath/ath10k/wmi-ops.h b/drivers/net/wireless/ath/ath10k/wmi-ops.h
index 64ebd304f907..b1d88fa60d11 100644
--- a/drivers/net/wireless/ath/ath10k/wmi-ops.h
+++ b/drivers/net/wireless/ath/ath10k/wmi-ops.h
@@ -194,6 +194,7 @@ struct wmi_ops {
 	struct sk_buff *(*gen_pdev_bss_chan_info_req)
 					(struct ath10k *ar,
 					 enum wmi_bss_survey_req_type type);
+	struct sk_buff *(*gen_echo)(struct ath10k *ar, u32 value);
 };
 
 int ath10k_wmi_cmd_send(struct ath10k *ar, struct sk_buff *skb, u32 cmd_id);
@@ -1382,4 +1383,20 @@ ath10k_wmi_pdev_bss_chan_info_request(struct ath10k *ar,
 				   wmi->cmd->pdev_bss_chan_info_request_cmdid);
 }
 
+static inline int
+ath10k_wmi_echo(struct ath10k *ar, u32 value)
+{
+	struct ath10k_wmi *wmi = &ar->wmi;
+	struct sk_buff *skb;
+
+	if (!wmi->ops->gen_echo)
+		return -EOPNOTSUPP;
+
+	skb = wmi->ops->gen_echo(ar, value);
+	if (IS_ERR(skb))
+		return PTR_ERR(skb);
+
+	return ath10k_wmi_cmd_send(ar, skb, wmi->cmd->echo_cmdid);
+}
+
 #endif
diff --git a/drivers/net/wireless/ath/ath10k/wmi-tlv.c b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
index e09337ee7c96..cd595855af36 100644
--- a/drivers/net/wireless/ath/ath10k/wmi-tlv.c
+++ b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
@@ -3081,6 +3081,34 @@ ath10k_wmi_tlv_op_gen_adaptive_qcs(struct ath10k *ar, bool enable)
 	return skb;
 }
 
+static struct sk_buff *
+ath10k_wmi_tlv_op_gen_echo(struct ath10k *ar, u32 value)
+{
+	struct wmi_echo_cmd *cmd;
+	struct wmi_tlv *tlv;
+	struct sk_buff *skb;
+	void *ptr;
+	size_t len;
+
+	len = sizeof(*tlv) + sizeof(*cmd);
+	skb = ath10k_wmi_alloc_skb(ar, len);
+	if (!skb)
+		return ERR_PTR(-ENOMEM);
+
+	ptr = (void *)skb->data;
+	tlv = ptr;
+	tlv->tag = __cpu_to_le16(WMI_TLV_TAG_STRUCT_ECHO_CMD);
+	tlv->len = __cpu_to_le16(sizeof(*cmd));
+	cmd = (void *)tlv->value;
+	cmd->value = cpu_to_le32(value);
+
+	ptr += sizeof(*tlv);
+	ptr += sizeof(*cmd);
+
+	ath10k_dbg(ar, ATH10K_DBG_WMI, "wmi tlv echo value 0x%08x\n", value);
+	return skb;
+}
+
 /****************/
 /* TLV mappings */
 /****************/
@@ -3485,6 +3513,7 @@ static const struct wmi_ops wmi_tlv_ops = {
 	.gen_adaptive_qcs = ath10k_wmi_tlv_op_gen_adaptive_qcs,
 	.fw_stats_fill = ath10k_wmi_main_op_fw_stats_fill,
 	.get_vdev_subtype = ath10k_wmi_op_get_vdev_subtype,
+	.gen_echo = ath10k_wmi_tlv_op_gen_echo,
 };
 
 static const struct wmi_peer_flags_map wmi_tlv_peer_flags_map = {
diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
index 169cd2e783eb..9ae4aacadb38 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -7649,6 +7649,24 @@ ath10k_wmi_10_4_ext_resource_config(struct ath10k *ar,
 	return skb;
 }
 
+static struct sk_buff *
+ath10k_wmi_op_gen_echo(struct ath10k *ar, u32 value)
+{
+	struct wmi_echo_cmd *cmd;
+	struct sk_buff *skb;
+
+	skb = ath10k_wmi_alloc_skb(ar, sizeof(*cmd));
+	if (!skb)
+		return ERR_PTR(-ENOMEM);
+
+	cmd = (struct wmi_echo_cmd *)skb->data;
+	cmd->value = cpu_to_le32(value);
+
+	ath10k_dbg(ar, ATH10K_DBG_WMI,
+		   "wmi echo value 0x%08x\n", value);
+	return skb;
+}
+
 static const struct wmi_ops wmi_ops = {
 	.rx = ath10k_wmi_op_rx,
 	.map_svc = wmi_main_svc_map,
@@ -7709,6 +7727,7 @@ static const struct wmi_ops wmi_ops = {
 	.gen_delba_send = ath10k_wmi_op_gen_delba_send,
 	.fw_stats_fill = ath10k_wmi_main_op_fw_stats_fill,
 	.get_vdev_subtype = ath10k_wmi_op_get_vdev_subtype,
+	.gen_echo = ath10k_wmi_op_gen_echo,
 	/* .gen_bcn_tmpl not implemented */
 	/* .gen_prb_tmpl not implemented */
 	/* .gen_p2p_go_bcn_ie not implemented */
@@ -7777,6 +7796,7 @@ static const struct wmi_ops wmi_10_1_ops = {
 	.gen_delba_send = ath10k_wmi_op_gen_delba_send,
 	.fw_stats_fill = ath10k_wmi_10x_op_fw_stats_fill,
 	.get_vdev_subtype = ath10k_wmi_op_get_vdev_subtype,
+	.gen_echo = ath10k_wmi_op_gen_echo,
 	/* .gen_bcn_tmpl not implemented */
 	/* .gen_prb_tmpl not implemented */
 	/* .gen_p2p_go_bcn_ie not implemented */
@@ -7796,6 +7816,7 @@ static const struct wmi_ops wmi_10_2_ops = {
 	.pull_svc_rdy = ath10k_wmi_10x_op_pull_svc_rdy_ev,
 	.gen_pdev_set_rd = ath10k_wmi_10x_op_gen_pdev_set_rd,
 	.gen_start_scan = ath10k_wmi_10x_op_gen_start_scan,
+	.gen_echo = ath10k_wmi_op_gen_echo,
 
 	.pull_scan = ath10k_wmi_op_pull_scan_ev,
 	.pull_mgmt_rx = ath10k_wmi_op_pull_mgmt_rx_ev,
@@ -7862,6 +7883,7 @@ static const struct wmi_ops wmi_10_2_4_ops = {
 	.pull_svc_rdy = ath10k_wmi_10x_op_pull_svc_rdy_ev,
 	.gen_pdev_set_rd = ath10k_wmi_10x_op_gen_pdev_set_rd,
 	.gen_start_scan = ath10k_wmi_10x_op_gen_start_scan,
+	.gen_echo = ath10k_wmi_op_gen_echo,
 
 	.pull_scan = ath10k_wmi_op_pull_scan_ev,
 	.pull_mgmt_rx = ath10k_wmi_op_pull_mgmt_rx_ev,
@@ -7984,6 +8006,7 @@ static const struct wmi_ops wmi_10_4_ops = {
 	.gen_pdev_get_temperature = ath10k_wmi_10_2_op_gen_pdev_get_temperature,
 	.get_vdev_subtype = ath10k_wmi_10_4_op_get_vdev_subtype,
 	.gen_pdev_bss_chan_info_req = ath10k_wmi_10_2_op_gen_pdev_bss_chan_info,
+	.gen_echo = ath10k_wmi_op_gen_echo,
 };
 
 int ath10k_wmi_attach(struct ath10k *ar)
-- 
2.1.4

[PATCH 1/4] ath10k: implement wmi echo command

[Michal Kazior]

Will be useful for implementing command barriers.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/wmi-ops.h | 17 +++++++++++++++++
 drivers/net/wireless/ath/ath10k/wmi-tlv.c | 29 +++++++++++++++++++++++++++++
 drivers/net/wireless/ath/ath10k/wmi.c     | 23 +++++++++++++++++++++++
 3 files changed, 69 insertions(+)

diff --git a/drivers/net/wireless/ath/ath10k/wmi-ops.h b/drivers/net/wireless/ath/ath10k/wmi-ops.h
index 64ebd304f907..b1d88fa60d11 100644
--- a/drivers/net/wireless/ath/ath10k/wmi-ops.h
+++ b/drivers/net/wireless/ath/ath10k/wmi-ops.h
@@ -194,6 +194,7 @@ struct wmi_ops {
 	struct sk_buff *(*gen_pdev_bss_chan_info_req)
 					(struct ath10k *ar,
 					 enum wmi_bss_survey_req_type type);
+	struct sk_buff *(*gen_echo)(struct ath10k *ar, u32 value);
 };
 
 int ath10k_wmi_cmd_send(struct ath10k *ar, struct sk_buff *skb, u32 cmd_id);
@@ -1382,4 +1383,20 @@ ath10k_wmi_pdev_bss_chan_info_request(struct ath10k *ar,
 				   wmi->cmd->pdev_bss_chan_info_request_cmdid);
 }
 
+static inline int
+ath10k_wmi_echo(struct ath10k *ar, u32 value)
+{
+	struct ath10k_wmi *wmi = &ar->wmi;
+	struct sk_buff *skb;
+
+	if (!wmi->ops->gen_echo)
+		return -EOPNOTSUPP;
+
+	skb = wmi->ops->gen_echo(ar, value);
+	if (IS_ERR(skb))
+		return PTR_ERR(skb);
+
+	return ath10k_wmi_cmd_send(ar, skb, wmi->cmd->echo_cmdid);
+}
+
 #endif
diff --git a/drivers/net/wireless/ath/ath10k/wmi-tlv.c b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
index e09337ee7c96..cd595855af36 100644
--- a/drivers/net/wireless/ath/ath10k/wmi-tlv.c
+++ b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
@@ -3081,6 +3081,34 @@ ath10k_wmi_tlv_op_gen_adaptive_qcs(struct ath10k *ar, bool enable)
 	return skb;
 }
 
+static struct sk_buff *
+ath10k_wmi_tlv_op_gen_echo(struct ath10k *ar, u32 value)
+{
+	struct wmi_echo_cmd *cmd;
+	struct wmi_tlv *tlv;
+	struct sk_buff *skb;
+	void *ptr;
+	size_t len;
+
+	len = sizeof(*tlv) + sizeof(*cmd);
+	skb = ath10k_wmi_alloc_skb(ar, len);
+	if (!skb)
+		return ERR_PTR(-ENOMEM);
+
+	ptr = (void *)skb->data;
+	tlv = ptr;
+	tlv->tag = __cpu_to_le16(WMI_TLV_TAG_STRUCT_ECHO_CMD);
+	tlv->len = __cpu_to_le16(sizeof(*cmd));
+	cmd = (void *)tlv->value;
+	cmd->value = cpu_to_le32(value);
+
+	ptr += sizeof(*tlv);
+	ptr += sizeof(*cmd);
+
+	ath10k_dbg(ar, ATH10K_DBG_WMI, "wmi tlv echo value 0x%08x\n", value);
+	return skb;
+}
+
 /****************/
 /* TLV mappings */
 /****************/
@@ -3485,6 +3513,7 @@ static const struct wmi_ops wmi_tlv_ops = {
 	.gen_adaptive_qcs = ath10k_wmi_tlv_op_gen_adaptive_qcs,
 	.fw_stats_fill = ath10k_wmi_main_op_fw_stats_fill,
 	.get_vdev_subtype = ath10k_wmi_op_get_vdev_subtype,
+	.gen_echo = ath10k_wmi_tlv_op_gen_echo,
 };
 
 static const struct wmi_peer_flags_map wmi_tlv_peer_flags_map = {
diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
index 169cd2e783eb..9ae4aacadb38 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -7649,6 +7649,24 @@ ath10k_wmi_10_4_ext_resource_config(struct ath10k *ar,
 	return skb;
 }
 
+static struct sk_buff *
+ath10k_wmi_op_gen_echo(struct ath10k *ar, u32 value)
+{
+	struct wmi_echo_cmd *cmd;
+	struct sk_buff *skb;
+
+	skb = ath10k_wmi_alloc_skb(ar, sizeof(*cmd));
+	if (!skb)
+		return ERR_PTR(-ENOMEM);
+
+	cmd = (struct wmi_echo_cmd *)skb->data;
+	cmd->value = cpu_to_le32(value);
+
+	ath10k_dbg(ar, ATH10K_DBG_WMI,
+		   "wmi echo value 0x%08x\n", value);
+	return skb;
+}
+
 static const struct wmi_ops wmi_ops = {
 	.rx = ath10k_wmi_op_rx,
 	.map_svc = wmi_main_svc_map,
@@ -7709,6 +7727,7 @@ static const struct wmi_ops wmi_ops = {
 	.gen_delba_send = ath10k_wmi_op_gen_delba_send,
 	.fw_stats_fill = ath10k_wmi_main_op_fw_stats_fill,
 	.get_vdev_subtype = ath10k_wmi_op_get_vdev_subtype,
+	.gen_echo = ath10k_wmi_op_gen_echo,
 	/* .gen_bcn_tmpl not implemented */
 	/* .gen_prb_tmpl not implemented */
 	/* .gen_p2p_go_bcn_ie not implemented */
@@ -7777,6 +7796,7 @@ static const struct wmi_ops wmi_10_1_ops = {
 	.gen_delba_send = ath10k_wmi_op_gen_delba_send,
 	.fw_stats_fill = ath10k_wmi_10x_op_fw_stats_fill,
 	.get_vdev_subtype = ath10k_wmi_op_get_vdev_subtype,
+	.gen_echo = ath10k_wmi_op_gen_echo,
 	/* .gen_bcn_tmpl not implemented */
 	/* .gen_prb_tmpl not implemented */
 	/* .gen_p2p_go_bcn_ie not implemented */
@@ -7796,6 +7816,7 @@ static const struct wmi_ops wmi_10_2_ops = {
 	.pull_svc_rdy = ath10k_wmi_10x_op_pull_svc_rdy_ev,
 	.gen_pdev_set_rd = ath10k_wmi_10x_op_gen_pdev_set_rd,
 	.gen_start_scan = ath10k_wmi_10x_op_gen_start_scan,
+	.gen_echo = ath10k_wmi_op_gen_echo,
 
 	.pull_scan = ath10k_wmi_op_pull_scan_ev,
 	.pull_mgmt_rx = ath10k_wmi_op_pull_mgmt_rx_ev,
@@ -7862,6 +7883,7 @@ static const struct wmi_ops wmi_10_2_4_ops = {
 	.pull_svc_rdy = ath10k_wmi_10x_op_pull_svc_rdy_ev,
 	.gen_pdev_set_rd = ath10k_wmi_10x_op_gen_pdev_set_rd,
 	.gen_start_scan = ath10k_wmi_10x_op_gen_start_scan,
+	.gen_echo = ath10k_wmi_op_gen_echo,
 
 	.pull_scan = ath10k_wmi_op_pull_scan_ev,
 	.pull_mgmt_rx = ath10k_wmi_op_pull_mgmt_rx_ev,
@@ -7984,6 +8006,7 @@ static const struct wmi_ops wmi_10_4_ops = {
 	.gen_pdev_get_temperature = ath10k_wmi_10_2_op_gen_pdev_get_temperature,
 	.get_vdev_subtype = ath10k_wmi_10_4_op_get_vdev_subtype,
 	.gen_pdev_bss_chan_info_req = ath10k_wmi_10_2_op_gen_pdev_bss_chan_info,
+	.gen_echo = ath10k_wmi_op_gen_echo,
 };
 
 int ath10k_wmi_attach(struct ath10k *ar)
-- 
2.1.4


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

[PATCH 2/4] ath10k: implement wmi echo event

[Michal Kazior]

Will be useful for implementing command barriers.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/wmi-ops.h | 12 ++++++++++++
 drivers/net/wireless/ath/ath10k/wmi-tlv.c | 28 ++++++++++++++++++++++++++++
 drivers/net/wireless/ath/ath10k/wmi.c     | 29 ++++++++++++++++++++++++++++-
 drivers/net/wireless/ath/ath10k/wmi.h     |  4 ++++
 4 files changed, 72 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath10k/wmi-ops.h b/drivers/net/wireless/ath/ath10k/wmi-ops.h
index b1d88fa60d11..c67eda78b69e 100644
--- a/drivers/net/wireless/ath/ath10k/wmi-ops.h
+++ b/drivers/net/wireless/ath/ath10k/wmi-ops.h
@@ -51,6 +51,8 @@ struct wmi_ops {
 			    struct wmi_roam_ev_arg *arg);
 	int (*pull_wow_event)(struct ath10k *ar, struct sk_buff *skb,
 			      struct wmi_wow_ev_arg *arg);
+	int (*pull_echo_ev)(struct ath10k *ar, struct sk_buff *skb,
+			    struct wmi_echo_ev_arg *arg);
 	enum wmi_txbf_conf (*get_txbf_conf_scheme)(struct ath10k *ar);
 
 	struct sk_buff *(*gen_pdev_suspend)(struct ath10k *ar, u32 suspend_opt);
@@ -350,6 +352,16 @@ ath10k_wmi_pull_wow_event(struct ath10k *ar, struct sk_buff *skb,
 	return ar->wmi.ops->pull_wow_event(ar, skb, arg);
 }
 
+static inline int
+ath10k_wmi_pull_echo_ev(struct ath10k *ar, struct sk_buff *skb,
+			struct wmi_echo_ev_arg *arg)
+{
+	if (!ar->wmi.ops->pull_echo_ev)
+		return -EOPNOTSUPP;
+
+	return ar->wmi.ops->pull_echo_ev(ar, skb, arg);
+}
+
 static inline enum wmi_txbf_conf
 ath10k_wmi_get_txbf_conf_scheme(struct ath10k *ar)
 {
diff --git a/drivers/net/wireless/ath/ath10k/wmi-tlv.c b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
index cd595855af36..a42f52dd9a36 100644
--- a/drivers/net/wireless/ath/ath10k/wmi-tlv.c
+++ b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
@@ -1223,6 +1223,33 @@ ath10k_wmi_tlv_op_pull_wow_ev(struct ath10k *ar, struct sk_buff *skb,
 	return 0;
 }
 
+static int ath10k_wmi_tlv_op_pull_echo_ev(struct ath10k *ar,
+					  struct sk_buff *skb,
+					  struct wmi_echo_ev_arg *arg)
+{
+	const void **tb;
+	const struct wmi_echo_event *ev;
+	int ret;
+
+	tb = ath10k_wmi_tlv_parse_alloc(ar, skb->data, skb->len, GFP_ATOMIC);
+	if (IS_ERR(tb)) {
+		ret = PTR_ERR(tb);
+		ath10k_warn(ar, "failed to parse tlv: %d\n", ret);
+		return ret;
+	}
+
+	ev = tb[WMI_TLV_TAG_STRUCT_ECHO_EVENT];
+	if (!ev) {
+		kfree(tb);
+		return -EPROTO;
+	}
+
+	arg->value = ev->value;
+
+	kfree(tb);
+	return 0;
+}
+
 static struct sk_buff *
 ath10k_wmi_tlv_op_gen_pdev_suspend(struct ath10k *ar, u32 opt)
 {
@@ -3457,6 +3484,7 @@ static const struct wmi_ops wmi_tlv_ops = {
 	.pull_fw_stats = ath10k_wmi_tlv_op_pull_fw_stats,
 	.pull_roam_ev = ath10k_wmi_tlv_op_pull_roam_ev,
 	.pull_wow_event = ath10k_wmi_tlv_op_pull_wow_ev,
+	.pull_echo_ev = ath10k_wmi_tlv_op_pull_echo_ev,
 	.get_txbf_conf_scheme = ath10k_wmi_tlv_txbf_conf_scheme,
 
 	.gen_pdev_suspend = ath10k_wmi_tlv_op_gen_pdev_suspend,
diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
index 9ae4aacadb38..8ca76ecd7e9b 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -2495,7 +2495,18 @@ exit:
 
 void ath10k_wmi_event_echo(struct ath10k *ar, struct sk_buff *skb)
 {
-	ath10k_dbg(ar, ATH10K_DBG_WMI, "WMI_ECHO_EVENTID\n");
+	struct wmi_echo_ev_arg arg = {};
+	int ret;
+
+	ret = ath10k_wmi_pull_echo_ev(ar, skb, &arg);
+	if (ret) {
+		ath10k_warn(ar, "failed to parse echo: %d\n", ret);
+		return;
+	}
+
+	ath10k_dbg(ar, ATH10K_DBG_WMI,
+		   "wmi event echo value 0x%08x\n",
+		   le32_to_cpu(arg.value));
 }
 
 int ath10k_wmi_event_debug_mesg(struct ath10k *ar, struct sk_buff *skb)
@@ -4792,6 +4803,17 @@ static int ath10k_wmi_op_pull_roam_ev(struct ath10k *ar, struct sk_buff *skb,
 	return 0;
 }
 
+static int ath10k_wmi_op_pull_echo_ev(struct ath10k *ar,
+				      struct sk_buff *skb,
+				      struct wmi_echo_ev_arg *arg)
+{
+	struct wmi_echo_event *ev = (void *)skb->data;
+
+	arg->value = ev->value;
+
+	return 0;
+}
+
 int ath10k_wmi_event_ready(struct ath10k *ar, struct sk_buff *skb)
 {
 	struct wmi_rdy_ev_arg arg = {};
@@ -7683,6 +7705,7 @@ static const struct wmi_ops wmi_ops = {
 	.pull_rdy = ath10k_wmi_op_pull_rdy_ev,
 	.pull_fw_stats = ath10k_wmi_main_op_pull_fw_stats,
 	.pull_roam_ev = ath10k_wmi_op_pull_roam_ev,
+	.pull_echo_ev = ath10k_wmi_op_pull_echo_ev,
 
 	.gen_pdev_suspend = ath10k_wmi_op_gen_pdev_suspend,
 	.gen_pdev_resume = ath10k_wmi_op_gen_pdev_resume,
@@ -7757,6 +7780,7 @@ static const struct wmi_ops wmi_10_1_ops = {
 	.pull_phyerr = ath10k_wmi_op_pull_phyerr_ev,
 	.pull_rdy = ath10k_wmi_op_pull_rdy_ev,
 	.pull_roam_ev = ath10k_wmi_op_pull_roam_ev,
+	.pull_echo_ev = ath10k_wmi_op_pull_echo_ev,
 
 	.gen_pdev_suspend = ath10k_wmi_op_gen_pdev_suspend,
 	.gen_pdev_resume = ath10k_wmi_op_gen_pdev_resume,
@@ -7828,6 +7852,7 @@ static const struct wmi_ops wmi_10_2_ops = {
 	.pull_phyerr = ath10k_wmi_op_pull_phyerr_ev,
 	.pull_rdy = ath10k_wmi_op_pull_rdy_ev,
 	.pull_roam_ev = ath10k_wmi_op_pull_roam_ev,
+	.pull_echo_ev = ath10k_wmi_op_pull_echo_ev,
 
 	.gen_pdev_suspend = ath10k_wmi_op_gen_pdev_suspend,
 	.gen_pdev_resume = ath10k_wmi_op_gen_pdev_resume,
@@ -7895,6 +7920,7 @@ static const struct wmi_ops wmi_10_2_4_ops = {
 	.pull_phyerr = ath10k_wmi_op_pull_phyerr_ev,
 	.pull_rdy = ath10k_wmi_op_pull_rdy_ev,
 	.pull_roam_ev = ath10k_wmi_op_pull_roam_ev,
+	.pull_echo_ev = ath10k_wmi_op_pull_echo_ev,
 
 	.gen_pdev_suspend = ath10k_wmi_op_gen_pdev_suspend,
 	.gen_pdev_resume = ath10k_wmi_op_gen_pdev_resume,
@@ -8002,6 +8028,7 @@ static const struct wmi_ops wmi_10_4_ops = {
 	.ext_resource_config = ath10k_wmi_10_4_ext_resource_config,
 
 	/* shared with 10.2 */
+	.pull_echo_ev = ath10k_wmi_op_pull_echo_ev,
 	.gen_request_stats = ath10k_wmi_op_gen_request_stats,
 	.gen_pdev_get_temperature = ath10k_wmi_10_2_op_gen_pdev_get_temperature,
 	.get_vdev_subtype = ath10k_wmi_10_4_op_get_vdev_subtype,
diff --git a/drivers/net/wireless/ath/ath10k/wmi.h b/drivers/net/wireless/ath/ath10k/wmi.h
index 3ef468893b3f..086d78807d2f 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.h
+++ b/drivers/net/wireless/ath/ath10k/wmi.h
@@ -6296,6 +6296,10 @@ struct wmi_roam_ev_arg {
 	__le32 rssi;
 };
 
+struct wmi_echo_ev_arg {
+	__le32 value;
+};
+
 struct wmi_pdev_temperature_event {
 	/* temperature value in Celcius degree */
 	__le32 temperature;
-- 
2.1.4

[PATCH 2/4] ath10k: implement wmi echo event

[Michal Kazior]

Will be useful for implementing command barriers.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/wmi-ops.h | 12 ++++++++++++
 drivers/net/wireless/ath/ath10k/wmi-tlv.c | 28 ++++++++++++++++++++++++++++
 drivers/net/wireless/ath/ath10k/wmi.c     | 29 ++++++++++++++++++++++++++++-
 drivers/net/wireless/ath/ath10k/wmi.h     |  4 ++++
 4 files changed, 72 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath10k/wmi-ops.h b/drivers/net/wireless/ath/ath10k/wmi-ops.h
index b1d88fa60d11..c67eda78b69e 100644
--- a/drivers/net/wireless/ath/ath10k/wmi-ops.h
+++ b/drivers/net/wireless/ath/ath10k/wmi-ops.h
@@ -51,6 +51,8 @@ struct wmi_ops {
 			    struct wmi_roam_ev_arg *arg);
 	int (*pull_wow_event)(struct ath10k *ar, struct sk_buff *skb,
 			      struct wmi_wow_ev_arg *arg);
+	int (*pull_echo_ev)(struct ath10k *ar, struct sk_buff *skb,
+			    struct wmi_echo_ev_arg *arg);
 	enum wmi_txbf_conf (*get_txbf_conf_scheme)(struct ath10k *ar);
 
 	struct sk_buff *(*gen_pdev_suspend)(struct ath10k *ar, u32 suspend_opt);
@@ -350,6 +352,16 @@ ath10k_wmi_pull_wow_event(struct ath10k *ar, struct sk_buff *skb,
 	return ar->wmi.ops->pull_wow_event(ar, skb, arg);
 }
 
+static inline int
+ath10k_wmi_pull_echo_ev(struct ath10k *ar, struct sk_buff *skb,
+			struct wmi_echo_ev_arg *arg)
+{
+	if (!ar->wmi.ops->pull_echo_ev)
+		return -EOPNOTSUPP;
+
+	return ar->wmi.ops->pull_echo_ev(ar, skb, arg);
+}
+
 static inline enum wmi_txbf_conf
 ath10k_wmi_get_txbf_conf_scheme(struct ath10k *ar)
 {
diff --git a/drivers/net/wireless/ath/ath10k/wmi-tlv.c b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
index cd595855af36..a42f52dd9a36 100644
--- a/drivers/net/wireless/ath/ath10k/wmi-tlv.c
+++ b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
@@ -1223,6 +1223,33 @@ ath10k_wmi_tlv_op_pull_wow_ev(struct ath10k *ar, struct sk_buff *skb,
 	return 0;
 }
 
+static int ath10k_wmi_tlv_op_pull_echo_ev(struct ath10k *ar,
+					  struct sk_buff *skb,
+					  struct wmi_echo_ev_arg *arg)
+{
+	const void **tb;
+	const struct wmi_echo_event *ev;
+	int ret;
+
+	tb = ath10k_wmi_tlv_parse_alloc(ar, skb->data, skb->len, GFP_ATOMIC);
+	if (IS_ERR(tb)) {
+		ret = PTR_ERR(tb);
+		ath10k_warn(ar, "failed to parse tlv: %d\n", ret);
+		return ret;
+	}
+
+	ev = tb[WMI_TLV_TAG_STRUCT_ECHO_EVENT];
+	if (!ev) {
+		kfree(tb);
+		return -EPROTO;
+	}
+
+	arg->value = ev->value;
+
+	kfree(tb);
+	return 0;
+}
+
 static struct sk_buff *
 ath10k_wmi_tlv_op_gen_pdev_suspend(struct ath10k *ar, u32 opt)
 {
@@ -3457,6 +3484,7 @@ static const struct wmi_ops wmi_tlv_ops = {
 	.pull_fw_stats = ath10k_wmi_tlv_op_pull_fw_stats,
 	.pull_roam_ev = ath10k_wmi_tlv_op_pull_roam_ev,
 	.pull_wow_event = ath10k_wmi_tlv_op_pull_wow_ev,
+	.pull_echo_ev = ath10k_wmi_tlv_op_pull_echo_ev,
 	.get_txbf_conf_scheme = ath10k_wmi_tlv_txbf_conf_scheme,
 
 	.gen_pdev_suspend = ath10k_wmi_tlv_op_gen_pdev_suspend,
diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
index 9ae4aacadb38..8ca76ecd7e9b 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -2495,7 +2495,18 @@ exit:
 
 void ath10k_wmi_event_echo(struct ath10k *ar, struct sk_buff *skb)
 {
-	ath10k_dbg(ar, ATH10K_DBG_WMI, "WMI_ECHO_EVENTID\n");
+	struct wmi_echo_ev_arg arg = {};
+	int ret;
+
+	ret = ath10k_wmi_pull_echo_ev(ar, skb, &arg);
+	if (ret) {
+		ath10k_warn(ar, "failed to parse echo: %d\n", ret);
+		return;
+	}
+
+	ath10k_dbg(ar, ATH10K_DBG_WMI,
+		   "wmi event echo value 0x%08x\n",
+		   le32_to_cpu(arg.value));
 }
 
 int ath10k_wmi_event_debug_mesg(struct ath10k *ar, struct sk_buff *skb)
@@ -4792,6 +4803,17 @@ static int ath10k_wmi_op_pull_roam_ev(struct ath10k *ar, struct sk_buff *skb,
 	return 0;
 }
 
+static int ath10k_wmi_op_pull_echo_ev(struct ath10k *ar,
+				      struct sk_buff *skb,
+				      struct wmi_echo_ev_arg *arg)
+{
+	struct wmi_echo_event *ev = (void *)skb->data;
+
+	arg->value = ev->value;
+
+	return 0;
+}
+
 int ath10k_wmi_event_ready(struct ath10k *ar, struct sk_buff *skb)
 {
 	struct wmi_rdy_ev_arg arg = {};
@@ -7683,6 +7705,7 @@ static const struct wmi_ops wmi_ops = {
 	.pull_rdy = ath10k_wmi_op_pull_rdy_ev,
 	.pull_fw_stats = ath10k_wmi_main_op_pull_fw_stats,
 	.pull_roam_ev = ath10k_wmi_op_pull_roam_ev,
+	.pull_echo_ev = ath10k_wmi_op_pull_echo_ev,
 
 	.gen_pdev_suspend = ath10k_wmi_op_gen_pdev_suspend,
 	.gen_pdev_resume = ath10k_wmi_op_gen_pdev_resume,
@@ -7757,6 +7780,7 @@ static const struct wmi_ops wmi_10_1_ops = {
 	.pull_phyerr = ath10k_wmi_op_pull_phyerr_ev,
 	.pull_rdy = ath10k_wmi_op_pull_rdy_ev,
 	.pull_roam_ev = ath10k_wmi_op_pull_roam_ev,
+	.pull_echo_ev = ath10k_wmi_op_pull_echo_ev,
 
 	.gen_pdev_suspend = ath10k_wmi_op_gen_pdev_suspend,
 	.gen_pdev_resume = ath10k_wmi_op_gen_pdev_resume,
@@ -7828,6 +7852,7 @@ static const struct wmi_ops wmi_10_2_ops = {
 	.pull_phyerr = ath10k_wmi_op_pull_phyerr_ev,
 	.pull_rdy = ath10k_wmi_op_pull_rdy_ev,
 	.pull_roam_ev = ath10k_wmi_op_pull_roam_ev,
+	.pull_echo_ev = ath10k_wmi_op_pull_echo_ev,
 
 	.gen_pdev_suspend = ath10k_wmi_op_gen_pdev_suspend,
 	.gen_pdev_resume = ath10k_wmi_op_gen_pdev_resume,
@@ -7895,6 +7920,7 @@ static const struct wmi_ops wmi_10_2_4_ops = {
 	.pull_phyerr = ath10k_wmi_op_pull_phyerr_ev,
 	.pull_rdy = ath10k_wmi_op_pull_rdy_ev,
 	.pull_roam_ev = ath10k_wmi_op_pull_roam_ev,
+	.pull_echo_ev = ath10k_wmi_op_pull_echo_ev,
 
 	.gen_pdev_suspend = ath10k_wmi_op_gen_pdev_suspend,
 	.gen_pdev_resume = ath10k_wmi_op_gen_pdev_resume,
@@ -8002,6 +8028,7 @@ static const struct wmi_ops wmi_10_4_ops = {
 	.ext_resource_config = ath10k_wmi_10_4_ext_resource_config,
 
 	/* shared with 10.2 */
+	.pull_echo_ev = ath10k_wmi_op_pull_echo_ev,
 	.gen_request_stats = ath10k_wmi_op_gen_request_stats,
 	.gen_pdev_get_temperature = ath10k_wmi_10_2_op_gen_pdev_get_temperature,
 	.get_vdev_subtype = ath10k_wmi_10_4_op_get_vdev_subtype,
diff --git a/drivers/net/wireless/ath/ath10k/wmi.h b/drivers/net/wireless/ath/ath10k/wmi.h
index 3ef468893b3f..086d78807d2f 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.h
+++ b/drivers/net/wireless/ath/ath10k/wmi.h
@@ -6296,6 +6296,10 @@ struct wmi_roam_ev_arg {
 	__le32 rssi;
 };
 
+struct wmi_echo_ev_arg {
+	__le32 value;
+};
+
 struct wmi_pdev_temperature_event {
 	/* temperature value in Celcius degree */
 	__le32 temperature;
-- 
2.1.4


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

[PATCH 3/4] ath10k: add wmi command barrier utility

[Michal Kazior]

This allows placing command barriers for explicit
serializing and synchronizing state.

Useful for future driver development.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/core.h |  1 +
 drivers/net/wireless/ath/ath10k/wmi.c  | 31 +++++++++++++++++++++++++++++++
 drivers/net/wireless/ath/ath10k/wmi.h  |  1 +
 3 files changed, 33 insertions(+)

diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index 9374bcde3d35..bf385052b168 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -142,6 +142,7 @@ struct ath10k_wmi {
 	enum ath10k_htc_ep_id eid;
 	struct completion service_ready;
 	struct completion unified_ready;
+	struct completion barrier;
 	wait_queue_head_t tx_credits_wq;
 	DECLARE_BITMAP(svc_map, WMI_SERVICE_MAX);
 	struct wmi_cmd_map *cmd;
diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
index 8ca76ecd7e9b..2bd9f2d0f186 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -29,6 +29,9 @@
 #include "p2p.h"
 #include "hw.h"
 
+#define ATH10K_WMI_BARRIER_ECHO_ID 0xBA991E9
+#define ATH10K_WMI_BARRIER_TIMEOUT_HZ (3 * HZ)
+
 /* MAIN WMI cmd track */
 static struct wmi_cmd_map wmi_cmd_map = {
 	.init_cmdid = WMI_INIT_CMDID,
@@ -2507,6 +2510,9 @@ void ath10k_wmi_event_echo(struct ath10k *ar, struct sk_buff *skb)
 	ath10k_dbg(ar, ATH10K_DBG_WMI,
 		   "wmi event echo value 0x%08x\n",
 		   le32_to_cpu(arg.value));
+
+	if (le32_to_cpu(arg.value) == ATH10K_WMI_BARRIER_ECHO_ID)
+		complete(&ar->wmi.barrier);
 }
 
 int ath10k_wmi_event_debug_mesg(struct ath10k *ar, struct sk_buff *skb)
@@ -7689,6 +7695,30 @@ ath10k_wmi_op_gen_echo(struct ath10k *ar, u32 value)
 	return skb;
 }
 
+int
+ath10k_wmi_barrier(struct ath10k *ar)
+{
+	int ret;
+	int time_left;
+
+	spin_lock_bh(&ar->data_lock);
+	reinit_completion(&ar->wmi.barrier);
+	spin_unlock_bh(&ar->data_lock);
+
+	ret = ath10k_wmi_echo(ar, ATH10K_WMI_BARRIER_ECHO_ID);
+	if (ret) {
+		ath10k_warn(ar, "failed to submit wmi echo: %d\n", ret);
+		return ret;
+	}
+
+	time_left = wait_for_completion_timeout(&ar->wmi.barrier,
+						ATH10K_WMI_BARRIER_TIMEOUT_HZ);
+	if (!time_left)
+		return -ETIMEDOUT;
+
+	return 0;
+}
+
 static const struct wmi_ops wmi_ops = {
 	.rx = ath10k_wmi_op_rx,
 	.map_svc = wmi_main_svc_map,
@@ -8086,6 +8116,7 @@ int ath10k_wmi_attach(struct ath10k *ar)
 
 	init_completion(&ar->wmi.service_ready);
 	init_completion(&ar->wmi.unified_ready);
+	init_completion(&ar->wmi.barrier);
 
 	INIT_WORK(&ar->svc_rdy_work, ath10k_wmi_event_service_ready_work);
 
diff --git a/drivers/net/wireless/ath/ath10k/wmi.h b/drivers/net/wireless/ath/ath10k/wmi.h
index 086d78807d2f..89adfa90ee8d 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.h
+++ b/drivers/net/wireless/ath/ath10k/wmi.h
@@ -6628,5 +6628,6 @@ void ath10k_wmi_10_4_op_fw_stats_fill(struct ath10k *ar,
 				      char *buf);
 int ath10k_wmi_op_get_vdev_subtype(struct ath10k *ar,
 				   enum wmi_vdev_subtype subtype);
+int ath10k_wmi_barrier(struct ath10k *ar);
 
 #endif /* _WMI_H_ */
-- 
2.1.4

[PATCH 3/4] ath10k: add wmi command barrier utility

[Michal Kazior]

This allows placing command barriers for explicit
serializing and synchronizing state.

Useful for future driver development.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/core.h |  1 +
 drivers/net/wireless/ath/ath10k/wmi.c  | 31 +++++++++++++++++++++++++++++++
 drivers/net/wireless/ath/ath10k/wmi.h  |  1 +
 3 files changed, 33 insertions(+)

diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index 9374bcde3d35..bf385052b168 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -142,6 +142,7 @@ struct ath10k_wmi {
 	enum ath10k_htc_ep_id eid;
 	struct completion service_ready;
 	struct completion unified_ready;
+	struct completion barrier;
 	wait_queue_head_t tx_credits_wq;
 	DECLARE_BITMAP(svc_map, WMI_SERVICE_MAX);
 	struct wmi_cmd_map *cmd;
diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
index 8ca76ecd7e9b..2bd9f2d0f186 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -29,6 +29,9 @@
 #include "p2p.h"
 #include "hw.h"
 
+#define ATH10K_WMI_BARRIER_ECHO_ID 0xBA991E9
+#define ATH10K_WMI_BARRIER_TIMEOUT_HZ (3 * HZ)
+
 /* MAIN WMI cmd track */
 static struct wmi_cmd_map wmi_cmd_map = {
 	.init_cmdid = WMI_INIT_CMDID,
@@ -2507,6 +2510,9 @@ void ath10k_wmi_event_echo(struct ath10k *ar, struct sk_buff *skb)
 	ath10k_dbg(ar, ATH10K_DBG_WMI,
 		   "wmi event echo value 0x%08x\n",
 		   le32_to_cpu(arg.value));
+
+	if (le32_to_cpu(arg.value) == ATH10K_WMI_BARRIER_ECHO_ID)
+		complete(&ar->wmi.barrier);
 }
 
 int ath10k_wmi_event_debug_mesg(struct ath10k *ar, struct sk_buff *skb)
@@ -7689,6 +7695,30 @@ ath10k_wmi_op_gen_echo(struct ath10k *ar, u32 value)
 	return skb;
 }
 
+int
+ath10k_wmi_barrier(struct ath10k *ar)
+{
+	int ret;
+	int time_left;
+
+	spin_lock_bh(&ar->data_lock);
+	reinit_completion(&ar->wmi.barrier);
+	spin_unlock_bh(&ar->data_lock);
+
+	ret = ath10k_wmi_echo(ar, ATH10K_WMI_BARRIER_ECHO_ID);
+	if (ret) {
+		ath10k_warn(ar, "failed to submit wmi echo: %d\n", ret);
+		return ret;
+	}
+
+	time_left = wait_for_completion_timeout(&ar->wmi.barrier,
+						ATH10K_WMI_BARRIER_TIMEOUT_HZ);
+	if (!time_left)
+		return -ETIMEDOUT;
+
+	return 0;
+}
+
 static const struct wmi_ops wmi_ops = {
 	.rx = ath10k_wmi_op_rx,
 	.map_svc = wmi_main_svc_map,
@@ -8086,6 +8116,7 @@ int ath10k_wmi_attach(struct ath10k *ar)
 
 	init_completion(&ar->wmi.service_ready);
 	init_completion(&ar->wmi.unified_ready);
+	init_completion(&ar->wmi.barrier);
 
 	INIT_WORK(&ar->svc_rdy_work, ath10k_wmi_event_service_ready_work);
 
diff --git a/drivers/net/wireless/ath/ath10k/wmi.h b/drivers/net/wireless/ath/ath10k/wmi.h
index 086d78807d2f..89adfa90ee8d 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.h
+++ b/drivers/net/wireless/ath/ath10k/wmi.h
@@ -6628,5 +6628,6 @@ void ath10k_wmi_10_4_op_fw_stats_fill(struct ath10k *ar,
 				      char *buf);
 int ath10k_wmi_op_get_vdev_subtype(struct ath10k *ar,
 				   enum wmi_vdev_subtype subtype);
+int ath10k_wmi_barrier(struct ath10k *ar);
 
 #endif /* _WMI_H_ */
-- 
2.1.4


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

[PATCH 4/4] ath10k: fix spurious tx/rx during boot

[Michal Kazior]

HW Rx filters and masks are not configured
properly by firmware during boot sequences. The
MAC_PCU_ADDR1 is set to 0s instead of 1s which
allows the HW to ACK any frame that passes through
MAC_PCU_RX_FILTER. The MAC_PCU_RX_FILTER itself
is misconfigured on boot as well.

The combination of these bugs ended up with the
following manifestations:
 - "no channel configured; ignoring frame(s)!"
   warnings in the driver
 - spurious ACKs (transmission) on the air during
   firmware bootup sequences

The former was a long standing and known bug
originally though mostly harmless.

However Marek recently discovered that this
problem also involves ACKing *all* frames the HW
receives (including beacons ;). Such frames
are delivered to host and generate the former
warning as well.

This could be a problem with regulatory compliance
in some rare cases (e.g. Taiwan which forbids
transmissions on channel 36 which is the default
bootup channel on 5Ghz band cards). The good news
is that it'd require someone else to violate
regulatory first to coerce our device to generate
and transmit an ACK.

The problem could be reproduced in a rather busy
environment that has a lot of APs. The likelihood
could be increased by injecting an msleep() of
5000 or longer immediately after
ath10k_htt_setup() in ath10k_core_start().

The reason why the former warnings were only
showing up seldom is because the device was either
quickly reset again (i.e. during firmware probing)
or wmi vdev was created (which fixes hw and fw
states).

It is technically possible for host driver to
override adequate hw registers however this can't
work reliably because the bug root cause lies in
incorrect firmware state on boot (internal
structure used to program MAC_PCU_ADDR1 is not
properly initialized) and only vdev create/delete
events can fix it. This is why the patch takes
dummy vdev approach.

This could be fixed in firmware as well but having
this fixed in driver is more robust, most notably
when thinking of users of older firmware such as
999.999.0.636.

Reported-by: Marek Puzyniak <marek.puzyniak@tieto.com>
Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/core.c | 68 ++++++++++++++++++++++++++++++++++
 1 file changed, 68 insertions(+)

diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index e88982921aa3..d2e255418d1b 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -1705,6 +1705,55 @@ static int ath10k_core_init_firmware_features(struct ath10k *ar)
 	return 0;
 }
 
+static int ath10k_core_reset_rx_filter(struct ath10k *ar)
+{
+	int ret;
+	int vdev_id;
+	int vdev_type;
+	int vdev_subtype;
+	const u8 *vdev_addr;
+
+	vdev_id = 0;
+	vdev_type = WMI_VDEV_TYPE_STA;
+	vdev_subtype = ath10k_wmi_get_vdev_subtype(ar, WMI_VDEV_SUBTYPE_NONE);
+	vdev_addr = ar->mac_addr;
+
+	ret = ath10k_wmi_vdev_create(ar, vdev_id, vdev_type, vdev_subtype,
+				     vdev_addr);
+	if (ret) {
+		ath10k_err(ar, "failed to create dummy vdev: %d\n", ret);
+		return ret;
+	}
+
+	ret = ath10k_wmi_vdev_delete(ar, vdev_id);
+	if (ret) {
+		ath10k_err(ar, "failed to delete dummy vdev: %d\n", ret);
+		return ret;
+	}
+
+	/* WMI and HTT may use separate HIF pipes and are not guaranteed to be
+	 * serialized properly implicitly.
+	 *
+	 * Moreover (most) WMI commands have no explicit acknowledges. It is
+	 * possible to infer it implicitly by poking firmware with echo
+	 * command - getting a reply means all preceding comments have been
+	 * (mostly) processed.
+	 *
+	 * In case of vdev create/delete this is sufficient.
+	 *
+	 * Without this it's possible to end up with a race when HTT Rx ring is
+	 * started before vdev create/delete hack is complete allowing a short
+	 * window of opportunity to receive (and Tx ACK) a bunch of frames.
+	 */
+	ret = ath10k_wmi_barrier(ar);
+	if (ret) {
+		ath10k_err(ar, "failed to ping firmware: %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
 int ath10k_core_start(struct ath10k *ar, enum ath10k_firmware_mode mode,
 		      const struct ath10k_fw_components *fw)
 {
@@ -1872,6 +1921,25 @@ int ath10k_core_start(struct ath10k *ar, enum ath10k_firmware_mode mode,
 		goto err_hif_stop;
 	}
 
+	/* Some firmware revisions do not properly set up hardware rx filter
+	 * registers.
+	 *
+	 * A known example from QCA9880 and 10.2.4 is that MAC_PCU_ADDR1_MASK
+	 * is filled with 0s instead of 1s allowing HW to respond with ACKs to
+	 * any frames that matches MAC_PCU_RX_FILTER which is also
+	 * misconfigured to accept anything.
+	 *
+	 * The ADDR1 is programmed using internal firmware structure field and
+	 * can't be (easily/sanely) reached from the driver explicitly. It is
+	 * possible to implicitly make it correct by creating a dummy vdev and
+	 * then deleting it.
+	 */
+	status = ath10k_core_reset_rx_filter(ar);
+	if (status) {
+		ath10k_err(ar, "failed to reset rx filter: %d\n", status);
+		goto err_hif_stop;
+	}
+
 	/* If firmware indicates Full Rx Reorder support it must be used in a
 	 * slightly different manner. Let HTT code know.
 	 */
-- 
2.1.4

[PATCH 4/4] ath10k: fix spurious tx/rx during boot

[Michal Kazior]

HW Rx filters and masks are not configured
properly by firmware during boot sequences. The
MAC_PCU_ADDR1 is set to 0s instead of 1s which
allows the HW to ACK any frame that passes through
MAC_PCU_RX_FILTER. The MAC_PCU_RX_FILTER itself
is misconfigured on boot as well.

The combination of these bugs ended up with the
following manifestations:
 - "no channel configured; ignoring frame(s)!"
   warnings in the driver
 - spurious ACKs (transmission) on the air during
   firmware bootup sequences

The former was a long standing and known bug
originally though mostly harmless.

However Marek recently discovered that this
problem also involves ACKing *all* frames the HW
receives (including beacons ;). Such frames
are delivered to host and generate the former
warning as well.

This could be a problem with regulatory compliance
in some rare cases (e.g. Taiwan which forbids
transmissions on channel 36 which is the default
bootup channel on 5Ghz band cards). The good news
is that it'd require someone else to violate
regulatory first to coerce our device to generate
and transmit an ACK.

The problem could be reproduced in a rather busy
environment that has a lot of APs. The likelihood
could be increased by injecting an msleep() of
5000 or longer immediately after
ath10k_htt_setup() in ath10k_core_start().

The reason why the former warnings were only
showing up seldom is because the device was either
quickly reset again (i.e. during firmware probing)
or wmi vdev was created (which fixes hw and fw
states).

It is technically possible for host driver to
override adequate hw registers however this can't
work reliably because the bug root cause lies in
incorrect firmware state on boot (internal
structure used to program MAC_PCU_ADDR1 is not
properly initialized) and only vdev create/delete
events can fix it. This is why the patch takes
dummy vdev approach.

This could be fixed in firmware as well but having
this fixed in driver is more robust, most notably
when thinking of users of older firmware such as
999.999.0.636.

Reported-by: Marek Puzyniak <marek.puzyniak@tieto.com>
Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/core.c | 68 ++++++++++++++++++++++++++++++++++
 1 file changed, 68 insertions(+)

diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index e88982921aa3..d2e255418d1b 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -1705,6 +1705,55 @@ static int ath10k_core_init_firmware_features(struct ath10k *ar)
 	return 0;
 }
 
+static int ath10k_core_reset_rx_filter(struct ath10k *ar)
+{
+	int ret;
+	int vdev_id;
+	int vdev_type;
+	int vdev_subtype;
+	const u8 *vdev_addr;
+
+	vdev_id = 0;
+	vdev_type = WMI_VDEV_TYPE_STA;
+	vdev_subtype = ath10k_wmi_get_vdev_subtype(ar, WMI_VDEV_SUBTYPE_NONE);
+	vdev_addr = ar->mac_addr;
+
+	ret = ath10k_wmi_vdev_create(ar, vdev_id, vdev_type, vdev_subtype,
+				     vdev_addr);
+	if (ret) {
+		ath10k_err(ar, "failed to create dummy vdev: %d\n", ret);
+		return ret;
+	}
+
+	ret = ath10k_wmi_vdev_delete(ar, vdev_id);
+	if (ret) {
+		ath10k_err(ar, "failed to delete dummy vdev: %d\n", ret);
+		return ret;
+	}
+
+	/* WMI and HTT may use separate HIF pipes and are not guaranteed to be
+	 * serialized properly implicitly.
+	 *
+	 * Moreover (most) WMI commands have no explicit acknowledges. It is
+	 * possible to infer it implicitly by poking firmware with echo
+	 * command - getting a reply means all preceding comments have been
+	 * (mostly) processed.
+	 *
+	 * In case of vdev create/delete this is sufficient.
+	 *
+	 * Without this it's possible to end up with a race when HTT Rx ring is
+	 * started before vdev create/delete hack is complete allowing a short
+	 * window of opportunity to receive (and Tx ACK) a bunch of frames.
+	 */
+	ret = ath10k_wmi_barrier(ar);
+	if (ret) {
+		ath10k_err(ar, "failed to ping firmware: %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
 int ath10k_core_start(struct ath10k *ar, enum ath10k_firmware_mode mode,
 		      const struct ath10k_fw_components *fw)
 {
@@ -1872,6 +1921,25 @@ int ath10k_core_start(struct ath10k *ar, enum ath10k_firmware_mode mode,
 		goto err_hif_stop;
 	}
 
+	/* Some firmware revisions do not properly set up hardware rx filter
+	 * registers.
+	 *
+	 * A known example from QCA9880 and 10.2.4 is that MAC_PCU_ADDR1_MASK
+	 * is filled with 0s instead of 1s allowing HW to respond with ACKs to
+	 * any frames that matches MAC_PCU_RX_FILTER which is also
+	 * misconfigured to accept anything.
+	 *
+	 * The ADDR1 is programmed using internal firmware structure field and
+	 * can't be (easily/sanely) reached from the driver explicitly. It is
+	 * possible to implicitly make it correct by creating a dummy vdev and
+	 * then deleting it.
+	 */
+	status = ath10k_core_reset_rx_filter(ar);
+	if (status) {
+		ath10k_err(ar, "failed to reset rx filter: %d\n", status);
+		goto err_hif_stop;
+	}
+
 	/* If firmware indicates Full Rx Reorder support it must be used in a
 	 * slightly different manner. Let HTT code know.
 	 */
-- 
2.1.4


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

Re: [PATCH 4/4] ath10k: fix spurious tx/rx during boot

[Ben Greear]

On 07/19/2016 03:34 AM, Michal Kazior wrote:
> HW Rx filters and masks are not configured
> properly by firmware during boot sequences. The
> MAC_PCU_ADDR1 is set to 0s instead of 1s which
> allows the HW to ACK any frame that passes through
> MAC_PCU_RX_FILTER. The MAC_PCU_RX_FILTER itself
> is misconfigured on boot as well.
>
> The combination of these bugs ended up with the
> following manifestations:
>  - "no channel configured; ignoring frame(s)!"
>    warnings in the driver
>  - spurious ACKs (transmission) on the air during
>    firmware bootup sequences
>
> The former was a long standing and known bug
> originally though mostly harmless.
>
> However Marek recently discovered that this
> problem also involves ACKing *all* frames the HW
> receives (including beacons ;). Such frames
> are delivered to host and generate the former
> warning as well.
>
> This could be a problem with regulatory compliance
> in some rare cases (e.g. Taiwan which forbids
> transmissions on channel 36 which is the default
> bootup channel on 5Ghz band cards). The good news
> is that it'd require someone else to violate
> regulatory first to coerce our device to generate
> and transmit an ACK.
>
> The problem could be reproduced in a rather busy
> environment that has a lot of APs. The likelihood
> could be increased by injecting an msleep() of
> 5000 or longer immediately after
> ath10k_htt_setup() in ath10k_core_start().
>
> The reason why the former warnings were only
> showing up seldom is because the device was either
> quickly reset again (i.e. during firmware probing)
> or wmi vdev was created (which fixes hw and fw
> states).
>
> It is technically possible for host driver to
> override adequate hw registers however this can't
> work reliably because the bug root cause lies in
> incorrect firmware state on boot (internal
> structure used to program MAC_PCU_ADDR1 is not
> properly initialized) and only vdev create/delete
> events can fix it. This is why the patch takes
> dummy vdev approach.
>
> This could be fixed in firmware as well but having
> this fixed in driver is more robust, most notably
> when thinking of users of older firmware such as
> 999.999.0.636.
>
> Reported-by: Marek Puzyniak <marek.puzyniak@tieto.com>
> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>

I was looking at firmware to make sure that I fixed what I could there....

 From what I can tell, 10.4 should not have this bug.  Did you see this only
on 10.1/10.2 firmware?  It is of course possible that I am mis-understanding
10.4....

Thanks,
Ben



-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: [PATCH 4/4] ath10k: fix spurious tx/rx during boot

[Ben Greear]

On 07/19/2016 03:34 AM, Michal Kazior wrote:
> HW Rx filters and masks are not configured
> properly by firmware during boot sequences. The
> MAC_PCU_ADDR1 is set to 0s instead of 1s which
> allows the HW to ACK any frame that passes through
> MAC_PCU_RX_FILTER. The MAC_PCU_RX_FILTER itself
> is misconfigured on boot as well.
>
> The combination of these bugs ended up with the
> following manifestations:
>  - "no channel configured; ignoring frame(s)!"
>    warnings in the driver
>  - spurious ACKs (transmission) on the air during
>    firmware bootup sequences
>
> The former was a long standing and known bug
> originally though mostly harmless.
>
> However Marek recently discovered that this
> problem also involves ACKing *all* frames the HW
> receives (including beacons ;). Such frames
> are delivered to host and generate the former
> warning as well.
>
> This could be a problem with regulatory compliance
> in some rare cases (e.g. Taiwan which forbids
> transmissions on channel 36 which is the default
> bootup channel on 5Ghz band cards). The good news
> is that it'd require someone else to violate
> regulatory first to coerce our device to generate
> and transmit an ACK.
>
> The problem could be reproduced in a rather busy
> environment that has a lot of APs. The likelihood
> could be increased by injecting an msleep() of
> 5000 or longer immediately after
> ath10k_htt_setup() in ath10k_core_start().
>
> The reason why the former warnings were only
> showing up seldom is because the device was either
> quickly reset again (i.e. during firmware probing)
> or wmi vdev was created (which fixes hw and fw
> states).
>
> It is technically possible for host driver to
> override adequate hw registers however this can't
> work reliably because the bug root cause lies in
> incorrect firmware state on boot (internal
> structure used to program MAC_PCU_ADDR1 is not
> properly initialized) and only vdev create/delete
> events can fix it. This is why the patch takes
> dummy vdev approach.
>
> This could be fixed in firmware as well but having
> this fixed in driver is more robust, most notably
> when thinking of users of older firmware such as
> 999.999.0.636.
>
> Reported-by: Marek Puzyniak <marek.puzyniak@tieto.com>
> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>

I was looking at firmware to make sure that I fixed what I could there....

 From what I can tell, 10.4 should not have this bug.  Did you see this only
on 10.1/10.2 firmware?  It is of course possible that I am mis-understanding
10.4....

Thanks,
Ben



-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com


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

Re: [PATCH 4/4] ath10k: fix spurious tx/rx during boot

[Michal Kazior]

On 24 August 2016 at 19:20, Ben Greear <greearb@candelatech.com> wrote:
> On 07/19/2016 03:34 AM, Michal Kazior wrote:
>>
>> HW Rx filters and masks are not configured
>> properly by firmware during boot sequences. The
>> MAC_PCU_ADDR1 is set to 0s instead of 1s which
>> allows the HW to ACK any frame that passes through
>> MAC_PCU_RX_FILTER. The MAC_PCU_RX_FILTER itself
>> is misconfigured on boot as well.
>>
>> The combination of these bugs ended up with the
>> following manifestations:
>>  - "no channel configured; ignoring frame(s)!"
>>    warnings in the driver
>>  - spurious ACKs (transmission) on the air during
>>    firmware bootup sequences
>>
>> The former was a long standing and known bug
>> originally though mostly harmless.
>>
>> However Marek recently discovered that this
>> problem also involves ACKing *all* frames the HW
>> receives (including beacons ;). Such frames
>> are delivered to host and generate the former
>> warning as well.
>>
>> This could be a problem with regulatory compliance
>> in some rare cases (e.g. Taiwan which forbids
>> transmissions on channel 36 which is the default
>> bootup channel on 5Ghz band cards). The good news
>> is that it'd require someone else to violate
>> regulatory first to coerce our device to generate
>> and transmit an ACK.
>>
>> The problem could be reproduced in a rather busy
>> environment that has a lot of APs. The likelihood
>> could be increased by injecting an msleep() of
>> 5000 or longer immediately after
>> ath10k_htt_setup() in ath10k_core_start().
>>
>> The reason why the former warnings were only
>> showing up seldom is because the device was either
>> quickly reset again (i.e. during firmware probing)
>> or wmi vdev was created (which fixes hw and fw
>> states).
>>
>> It is technically possible for host driver to
>> override adequate hw registers however this can't
>> work reliably because the bug root cause lies in
>> incorrect firmware state on boot (internal
>> structure used to program MAC_PCU_ADDR1 is not
>> properly initialized) and only vdev create/delete
>> events can fix it. This is why the patch takes
>> dummy vdev approach.
>>
>> This could be fixed in firmware as well but having
>> this fixed in driver is more robust, most notably
>> when thinking of users of older firmware such as
>> 999.999.0.636.
>>
>> Reported-by: Marek Puzyniak <marek.puzyniak@tieto.com>
>> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
>
>
> I was looking at firmware to make sure that I fixed what I could there...=
.
>
> From what I can tell, 10.4 should not have this bug.  Did you see this on=
ly
> on 10.1/10.2 firmware?  It is of course possible that I am mis-understand=
ing
> 10.4....

I did see it on 10.1 and 10.2. Don't recall seeing it on 10.4 though.
If you didn't see warnings on 10.4 even after adding msleep() as per
commit log then I guess it doesn't suffer from the bug.


Micha=C5=82

Re: [PATCH 4/4] ath10k: fix spurious tx/rx during boot

[Michal Kazior]

On 24 August 2016 at 19:20, Ben Greear <greearb@candelatech.com> wrote:
> On 07/19/2016 03:34 AM, Michal Kazior wrote:
>>
>> HW Rx filters and masks are not configured
>> properly by firmware during boot sequences. The
>> MAC_PCU_ADDR1 is set to 0s instead of 1s which
>> allows the HW to ACK any frame that passes through
>> MAC_PCU_RX_FILTER. The MAC_PCU_RX_FILTER itself
>> is misconfigured on boot as well.
>>
>> The combination of these bugs ended up with the
>> following manifestations:
>>  - "no channel configured; ignoring frame(s)!"
>>    warnings in the driver
>>  - spurious ACKs (transmission) on the air during
>>    firmware bootup sequences
>>
>> The former was a long standing and known bug
>> originally though mostly harmless.
>>
>> However Marek recently discovered that this
>> problem also involves ACKing *all* frames the HW
>> receives (including beacons ;). Such frames
>> are delivered to host and generate the former
>> warning as well.
>>
>> This could be a problem with regulatory compliance
>> in some rare cases (e.g. Taiwan which forbids
>> transmissions on channel 36 which is the default
>> bootup channel on 5Ghz band cards). The good news
>> is that it'd require someone else to violate
>> regulatory first to coerce our device to generate
>> and transmit an ACK.
>>
>> The problem could be reproduced in a rather busy
>> environment that has a lot of APs. The likelihood
>> could be increased by injecting an msleep() of
>> 5000 or longer immediately after
>> ath10k_htt_setup() in ath10k_core_start().
>>
>> The reason why the former warnings were only
>> showing up seldom is because the device was either
>> quickly reset again (i.e. during firmware probing)
>> or wmi vdev was created (which fixes hw and fw
>> states).
>>
>> It is technically possible for host driver to
>> override adequate hw registers however this can't
>> work reliably because the bug root cause lies in
>> incorrect firmware state on boot (internal
>> structure used to program MAC_PCU_ADDR1 is not
>> properly initialized) and only vdev create/delete
>> events can fix it. This is why the patch takes
>> dummy vdev approach.
>>
>> This could be fixed in firmware as well but having
>> this fixed in driver is more robust, most notably
>> when thinking of users of older firmware such as
>> 999.999.0.636.
>>
>> Reported-by: Marek Puzyniak <marek.puzyniak@tieto.com>
>> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
>
>
> I was looking at firmware to make sure that I fixed what I could there....
>
> From what I can tell, 10.4 should not have this bug.  Did you see this only
> on 10.1/10.2 firmware?  It is of course possible that I am mis-understanding
> 10.4....

I did see it on 10.1 and 10.2. Don't recall seeing it on 10.4 though.
If you didn't see warnings on 10.4 even after adding msleep() as per
commit log then I guess it doesn't suffer from the bug.


Michał

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

Re: [PATCH 4/4] ath10k: fix spurious tx/rx during boot

[Ben Greear]

On 08/24/2016 11:18 PM, Michal Kazior wrote:

>> I was looking at firmware to make sure that I fixed what I could there....
>>
>> From what I can tell, 10.4 should not have this bug.  Did you see this only
>> on 10.1/10.2 firmware?  It is of course possible that I am mis-understanding
>> 10.4....
>
> I did see it on 10.1 and 10.2. Don't recall seeing it on 10.4 though.
> If you didn't see warnings on 10.4 even after adding msleep() as per
> commit log then I guess it doesn't suffer from the bug.

I can still occasionally see that message with a 15000 ms sleep.

Based on debugging, it seems my firmware is now setting the mac-mask properly.

But, as you mention, the rxfilter is enabled very early.  So, probably
it is still possible to see packets early if they are multicast, bcast, etc.

I don't think it is worth re-working the entire rx-filter calc in
the concurrency logic properly for 10.1 firmware, so I'm going to figure
my fix is good enough as is as long as it sets the mac-mask properly.

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: [PATCH 4/4] ath10k: fix spurious tx/rx during boot

[Ben Greear]

On 08/24/2016 11:18 PM, Michal Kazior wrote:

>> I was looking at firmware to make sure that I fixed what I could there....
>>
>> From what I can tell, 10.4 should not have this bug.  Did you see this only
>> on 10.1/10.2 firmware?  It is of course possible that I am mis-understanding
>> 10.4....
>
> I did see it on 10.1 and 10.2. Don't recall seeing it on 10.4 though.
> If you didn't see warnings on 10.4 even after adding msleep() as per
> commit log then I guess it doesn't suffer from the bug.

I can still occasionally see that message with a 15000 ms sleep.

Based on debugging, it seems my firmware is now setting the mac-mask properly.

But, as you mention, the rxfilter is enabled very early.  So, probably
it is still possible to see packets early if they are multicast, bcast, etc.

I don't think it is worth re-working the entire rx-filter calc in
the concurrency logic properly for 10.1 firmware, so I'm going to figure
my fix is good enough as is as long as it sets the mac-mask properly.

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com


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

Re: [1/4] ath10k: implement wmi echo command

[Kalle Valo]

Michal Kazior <michal.kazior@tieto.com> wrote:
> Will be useful for implementing command barriers.
> 
> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>

Thanks, 4 patches applied to ath-next branch of ath.git:

e25854f2404c ath10k: implement wmi echo command
84d4911b7184 ath10k: implement wmi echo event
20ddca21dcf8 ath10k: add wmi command barrier utility
47b1848d9fde ath10k: fix spurious tx/rx during boot

-- 
Sent by pwcli
https://patchwork.kernel.org/patch/9236705/

Re: [1/4] ath10k: implement wmi echo command

[Kalle Valo]

Michal Kazior <michal.kazior@tieto.com> wrote:
> Will be useful for implementing command barriers.
> 
> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>

Thanks, 4 patches applied to ath-next branch of ath.git:

e25854f2404c ath10k: implement wmi echo command
84d4911b7184 ath10k: implement wmi echo event
20ddca21dcf8 ath10k: add wmi command barrier utility
47b1848d9fde ath10k: fix spurious tx/rx during boot

-- 
Sent by pwcli
https://patchwork.kernel.org/patch/9236705/


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

Re: [PATCH 4/4] ath10k: fix spurious tx/rx during boot

[Hsu, Ryan]

On 07/19/2016 03:34 AM, Michal Kazior wrote:
>  =20
> +static int ath10k_core_reset_rx_filter(struct ath10k *ar)
> +{
> +	int ret;
> +	int vdev_id;
> +	int vdev_type;
> +	int vdev_subtype;
> +	const u8 *vdev_addr;
> +
> +	vdev_id =3D 0;
> +	vdev_type =3D WMI_VDEV_TYPE_STA;
> +	vdev_subtype =3D ath10k_wmi_get_vdev_subtype(ar, WMI_VDEV_SUBTYPE_NONE)=
;
> +	vdev_addr =3D ar->mac_addr;
> +
> +	ret =3D ath10k_wmi_vdev_create(ar, vdev_id, vdev_type, vdev_subtype,
> +				     vdev_addr);
> +	if (ret) {
> +		ath10k_err(ar, "failed to create dummy vdev: %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret =3D ath10k_wmi_vdev_delete(ar, vdev_id);
> +	if (ret) {
> +		ath10k_err(ar, "failed to delete dummy vdev: %d\n", ret);
> +		return ret;
> +	}
> +
> +	/* WMI and HTT may use separate HIF pipes and are not guaranteed to be
> +	 * serialized properly implicitly.
> +	 *
> +	 * Moreover (most) WMI commands have no explicit acknowledges. It is
> +	 * possible to infer it implicitly by poking firmware with echo
> +	 * command - getting a reply means all preceding comments have been
> +	 * (mostly) processed.
> +	 *
> +	 * In case of vdev create/delete this is sufficient.
> +	 *
> +	 * Without this it's possible to end up with a race when HTT Rx ring is
> +	 * started before vdev create/delete hack is complete allowing a short
> +	 * window of opportunity to receive (and Tx ACK) a bunch of frames.
> +	 */
> +	ret =3D ath10k_wmi_barrier(ar);
QCA6174 UTF firmware seems doesn't support the WMI_ECHO command.

[16460.274822] ath10k_pci 0000:04:00.0: wmi tlv echo value 0x0ba991e9
...
[16463.461970] ath10k_pci 0000:04:00.0: failed to ping firmware: -110
[16463.461975] ath10k_pci 0000:04:00.0: failed to reset rx filter: -110

Has anyone verified any AP solution to see if UTF mode is still working=20
with after this patch?

Anyway, I would like to exclude the workaround from all solution's UTF mode=
.

Michal any concerns? (or maybe just for QCA61x4 if any...)

> +	if (ret) {
> +		ath10k_err(ar, "failed to ping firmware: %d\n", ret);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
>

Re: [PATCH 4/4] ath10k: fix spurious tx/rx during boot

[Hsu, Ryan]

On 07/19/2016 03:34 AM, Michal Kazior wrote:
>   
> +static int ath10k_core_reset_rx_filter(struct ath10k *ar)
> +{
> +	int ret;
> +	int vdev_id;
> +	int vdev_type;
> +	int vdev_subtype;
> +	const u8 *vdev_addr;
> +
> +	vdev_id = 0;
> +	vdev_type = WMI_VDEV_TYPE_STA;
> +	vdev_subtype = ath10k_wmi_get_vdev_subtype(ar, WMI_VDEV_SUBTYPE_NONE);
> +	vdev_addr = ar->mac_addr;
> +
> +	ret = ath10k_wmi_vdev_create(ar, vdev_id, vdev_type, vdev_subtype,
> +				     vdev_addr);
> +	if (ret) {
> +		ath10k_err(ar, "failed to create dummy vdev: %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = ath10k_wmi_vdev_delete(ar, vdev_id);
> +	if (ret) {
> +		ath10k_err(ar, "failed to delete dummy vdev: %d\n", ret);
> +		return ret;
> +	}
> +
> +	/* WMI and HTT may use separate HIF pipes and are not guaranteed to be
> +	 * serialized properly implicitly.
> +	 *
> +	 * Moreover (most) WMI commands have no explicit acknowledges. It is
> +	 * possible to infer it implicitly by poking firmware with echo
> +	 * command - getting a reply means all preceding comments have been
> +	 * (mostly) processed.
> +	 *
> +	 * In case of vdev create/delete this is sufficient.
> +	 *
> +	 * Without this it's possible to end up with a race when HTT Rx ring is
> +	 * started before vdev create/delete hack is complete allowing a short
> +	 * window of opportunity to receive (and Tx ACK) a bunch of frames.
> +	 */
> +	ret = ath10k_wmi_barrier(ar);
QCA6174 UTF firmware seems doesn't support the WMI_ECHO command.

[16460.274822] ath10k_pci 0000:04:00.0: wmi tlv echo value 0x0ba991e9
...
[16463.461970] ath10k_pci 0000:04:00.0: failed to ping firmware: -110
[16463.461975] ath10k_pci 0000:04:00.0: failed to reset rx filter: -110

Has anyone verified any AP solution to see if UTF mode is still working 
with after this patch?

Anyway, I would like to exclude the workaround from all solution's UTF mode.

Michal any concerns? (or maybe just for QCA61x4 if any...)

> +	if (ret) {
> +		ath10k_err(ar, "failed to ping firmware: %d\n", ret);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
>

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

Re: [PATCH 4/4] ath10k: fix spurious tx/rx during boot

[Michal Kazior]

On 17 September 2016 at 00:37, Hsu, Ryan <ryanhsu@qca.qualcomm.com> wrote:
[...]
>> +     /* WMI and HTT may use separate HIF pipes and are not guaranteed to be
>> +      * serialized properly implicitly.
>> +      *
>> +      * Moreover (most) WMI commands have no explicit acknowledges. It is
>> +      * possible to infer it implicitly by poking firmware with echo
>> +      * command - getting a reply means all preceding comments have been
>> +      * (mostly) processed.
>> +      *
>> +      * In case of vdev create/delete this is sufficient.
>> +      *
>> +      * Without this it's possible to end up with a race when HTT Rx ring is
>> +      * started before vdev create/delete hack is complete allowing a short
>> +      * window of opportunity to receive (and Tx ACK) a bunch of frames.
>> +      */
>> +     ret = ath10k_wmi_barrier(ar);
> QCA6174 UTF firmware seems doesn't support the WMI_ECHO command.
>
> [16460.274822] ath10k_pci 0000:04:00.0: wmi tlv echo value 0x0ba991e9
> ...
> [16463.461970] ath10k_pci 0000:04:00.0: failed to ping firmware: -110
> [16463.461975] ath10k_pci 0000:04:00.0: failed to reset rx filter: -110
>
> Has anyone verified any AP solution to see if UTF mode is still working
> with after this patch?
>
> Anyway, I would like to exclude the workaround from all solution's UTF mode.
>
> Michal any concerns? (or maybe just for QCA61x4 if any...)

I didn't expect UTF wouldn't support echo.. Sorry!

If you skip this workaround for UTF I guess the device will (again) be
able to generate some bogus traffic on boot for UTF case. Not sure how
much of a problem that is (assuming it is at all).


Michal

Re: [PATCH 4/4] ath10k: fix spurious tx/rx during boot

[Michal Kazior]

On 17 September 2016 at 00:37, Hsu, Ryan <ryanhsu@qca.qualcomm.com> wrote:
[...]
>> +     /* WMI and HTT may use separate HIF pipes and are not guaranteed to be
>> +      * serialized properly implicitly.
>> +      *
>> +      * Moreover (most) WMI commands have no explicit acknowledges. It is
>> +      * possible to infer it implicitly by poking firmware with echo
>> +      * command - getting a reply means all preceding comments have been
>> +      * (mostly) processed.
>> +      *
>> +      * In case of vdev create/delete this is sufficient.
>> +      *
>> +      * Without this it's possible to end up with a race when HTT Rx ring is
>> +      * started before vdev create/delete hack is complete allowing a short
>> +      * window of opportunity to receive (and Tx ACK) a bunch of frames.
>> +      */
>> +     ret = ath10k_wmi_barrier(ar);
> QCA6174 UTF firmware seems doesn't support the WMI_ECHO command.
>
> [16460.274822] ath10k_pci 0000:04:00.0: wmi tlv echo value 0x0ba991e9
> ...
> [16463.461970] ath10k_pci 0000:04:00.0: failed to ping firmware: -110
> [16463.461975] ath10k_pci 0000:04:00.0: failed to reset rx filter: -110
>
> Has anyone verified any AP solution to see if UTF mode is still working
> with after this patch?
>
> Anyway, I would like to exclude the workaround from all solution's UTF mode.
>
> Michal any concerns? (or maybe just for QCA61x4 if any...)

I didn't expect UTF wouldn't support echo.. Sorry!

If you skip this workaround for UTF I guess the device will (again) be
able to generate some bogus traffic on boot for UTF case. Not sure how
much of a problem that is (assuming it is at all).


Michal

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