[ath9k-devel] [PATCH 0/4] ath10k: fixes

[ath9k-devel] [PATCH 0/4] ath10k: fixes

[Michal Kazior]

This is part 1 of 4 of a bigger patchset.
Split for easier review.

Contains functional fixes.

Michal Kazior (4):
  ath10k: fix teardown ordering
  ath10k: fix possible deadlock
  ath10k: setup rts/frag thresholds upon vdev creation
  ath10k: do not setup rts/frag thresholds for suspended interfaces

 drivers/net/wireless/ath/ath10k/mac.c |   30 +++++++++++++++++++++++++-----
 drivers/net/wireless/ath/ath10k/pci.c |    2 +-
 2 files changed, 26 insertions(+), 6 deletions(-)

-- 
1.7.9.5

[ath9k-devel] [PATCH 1/4] ath10k: fix teardown ordering

[Michal Kazior]

This should fix memory corruption if HIF is tried
to be restarted.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/pci.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index 8e4e832..1bd381f 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -1263,7 +1263,6 @@ static void ath10k_pci_hif_stop(struct ath10k *ar)
 	ath10k_pci_process_ce(ar);
 	ath10k_pci_cleanup_ce(ar);
 	ath10k_pci_buffer_cleanup(ar);
-	ath10k_pci_ce_deinit(ar);
 }
 
 static int ath10k_pci_hif_exchange_bmi_msg(struct ath10k *ar,
@@ -2332,6 +2331,7 @@ static void ath10k_pci_remove(struct pci_dev *pdev)
 	tasklet_kill(&ar_pci->msi_fw_err);
 
 	ath10k_core_unregister(ar);
+	ath10k_pci_ce_deinit(ar);
 	ath10k_pci_stop_intr(ar);
 
 	pci_set_drvdata(pdev, NULL);
-- 
1.7.9.5

[ath9k-devel] [PATCH 2/4] ath10k: fix possible deadlock

[Michal Kazior]

It was possible to have a deadlock due to inverted
locking of local->iflist_mtx and
ath10k->conf_mutex.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/mac.c |   10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 1285554..9943ee9 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -2578,8 +2578,9 @@ static int ath10k_set_rts_threshold(struct ieee80211_hw *hw, u32 value)
 	ar_iter.ar = ar;
 
 	mutex_lock(&ar->conf_mutex);
-	ieee80211_iterate_active_interfaces(hw, IEEE80211_IFACE_ITER_RESUME_ALL,
-					    ath10k_set_rts_iter, &ar_iter);
+	ieee80211_iterate_active_interfaces_atomic(
+		hw, IEEE80211_IFACE_ITER_RESUME_ALL,
+		ath10k_set_rts_iter, &ar_iter);
 	mutex_unlock(&ar->conf_mutex);
 
 	return ar_iter.ret;
@@ -2619,8 +2620,9 @@ static int ath10k_set_frag_threshold(struct ieee80211_hw *hw, u32 value)
 	ar_iter.ar = ar;
 
 	mutex_lock(&ar->conf_mutex);
-	ieee80211_iterate_active_interfaces(hw, IEEE80211_IFACE_ITER_RESUME_ALL,
-					    ath10k_set_frag_iter, &ar_iter);
+	ieee80211_iterate_active_interfaces_atomic(
+		hw, IEEE80211_IFACE_ITER_RESUME_ALL,
+		ath10k_set_frag_iter, &ar_iter);
 	mutex_unlock(&ar->conf_mutex);
 
 	return ar_iter.ret;
-- 
1.7.9.5

[ath9k-devel] [PATCH 3/4] ath10k: setup rts/frag thresholds upon vdev creation

[Michal Kazior]

mac80211 configures rts/frag thresholds per-hw not
per-vif. ath10k FW expects those values to be set
per-vdev (i.e. per-vif).

ath10k should now respect rts/frag thresholds set
before a given interface was brought up.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/mac.c |   20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 9943ee9..ff0bb38 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -1753,7 +1753,7 @@ static int ath10k_add_interface(struct ieee80211_hw *hw,
 	struct ath10k_vif *arvif = ath10k_vif_to_arvif(vif);
 	enum wmi_sta_powersave_param param;
 	int ret = 0;
-	u32 value;
+	u32 value, rts, frag;
 	int bit;
 
 	mutex_lock(&ar->conf_mutex);
@@ -1856,6 +1856,24 @@ static int ath10k_add_interface(struct ieee80211_hw *hw,
 			ath10k_warn("Failed to set PSPOLL count: %d\n", ret);
 	}
 
+	rts = min_t(u32, ar->hw->wiphy->rts_threshold, ATH10K_RTS_MAX);
+	ret = ath10k_wmi_vdev_set_param(ar, arvif->vdev_id,
+					 WMI_VDEV_PARAM_RTS_THRESHOLD,
+					 rts);
+	if (ret)
+		ath10k_warn("failed to set rts threshold for vdev %d (%d)\n",
+			    arvif->vdev_id, ret);
+
+	frag = clamp_t(u32, ar->hw->wiphy->frag_threshold,
+		       ATH10K_FRAGMT_THRESHOLD_MIN,
+		       ATH10K_FRAGMT_THRESHOLD_MAX);
+	ret = ath10k_wmi_vdev_set_param(ar, arvif->vdev_id,
+					WMI_VDEV_PARAM_FRAGMENTATION_THRESHOLD,
+					frag);
+	if (ret)
+		ath10k_warn("failed to set frag threshold for vdev %d (%d)\n",
+			    arvif->vdev_id, ret);
+
 	if (arvif->vdev_type == WMI_VDEV_TYPE_MONITOR)
 		ar->monitor_present = true;
 
-- 
1.7.9.5

[ath9k-devel] [PATCH 4/4] ath10k: do not setup rts/frag thresholds for suspended interfaces

[Michal Kazior]

mac80211 calls for rts/frag threshold hooks before
any interface is brought back up again when
resuming.

We would set vdev parameters before given vdev is
created lading to a FW crash.

rts/frag thresholds will be re-set accordingly in
add_interface() hook anyway.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/mac.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index ff0bb38..cdee800 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -2597,7 +2597,7 @@ static int ath10k_set_rts_threshold(struct ieee80211_hw *hw, u32 value)
 
 	mutex_lock(&ar->conf_mutex);
 	ieee80211_iterate_active_interfaces_atomic(
-		hw, IEEE80211_IFACE_ITER_RESUME_ALL,
+		hw, IEEE80211_IFACE_ITER_NORMAL,
 		ath10k_set_rts_iter, &ar_iter);
 	mutex_unlock(&ar->conf_mutex);
 
@@ -2639,7 +2639,7 @@ static int ath10k_set_frag_threshold(struct ieee80211_hw *hw, u32 value)
 
 	mutex_lock(&ar->conf_mutex);
 	ieee80211_iterate_active_interfaces_atomic(
-		hw, IEEE80211_IFACE_ITER_RESUME_ALL,
+		hw, IEEE80211_IFACE_ITER_NORMAL,
 		ath10k_set_frag_iter, &ar_iter);
 	mutex_unlock(&ar->conf_mutex);
 
-- 
1.7.9.5

[ath9k-devel] [PATCH 3/4] ath10k: setup rts/frag thresholds upon vdev creation

[Sujith Manoharan]

Michal Kazior wrote:
> +	frag = clamp_t(u32, ar->hw->wiphy->frag_threshold,
> +		       ATH10K_FRAGMT_THRESHOLD_MIN,
> +		       ATH10K_FRAGMT_THRESHOLD_MAX);
> +	ret = ath10k_wmi_vdev_set_param(ar, arvif->vdev_id,
> +					WMI_VDEV_PARAM_FRAGMENTATION_THRESHOLD,
> +					frag);

Looks like the FW doesn't handle WMI_VDEV_PARAM_FRAGMENTATION_THRESHOLD at all.

Sujith

[ath9k-devel] [PATCH 0/4] ath10k: fixes

[Sujith Manoharan]

Michal Kazior wrote:
> This is part 1 of 4 of a bigger patchset.
> Split for easier review.
> 
> Contains functional fixes.
> 
> Michal Kazior (4):
>   ath10k: fix teardown ordering
>   ath10k: fix possible deadlock
>   ath10k: setup rts/frag thresholds upon vdev creation
>   ath10k: do not setup rts/frag thresholds for suspended interfaces
> 
>  drivers/net/wireless/ath/ath10k/mac.c |   30 +++++++++++++++++++++++++-----
>  drivers/net/wireless/ath/ath10k/pci.c |    2 +-
>  2 files changed, 26 insertions(+), 6 deletions(-)

Other than the observation about an incomplete FW feature,
this looks good to me.

Sujith

[ath9k-devel] [PATCH 0/4] ath10k: fixes

[Kalle Valo]

Sujith Manoharan <sujith@msujith.org> writes:

> Michal Kazior wrote:
>> This is part 1 of 4 of a bigger patchset.
>> Split for easier review.
>> 
>> Contains functional fixes.
>> 
>> Michal Kazior (4):
>>   ath10k: fix teardown ordering
>>   ath10k: fix possible deadlock
>>   ath10k: setup rts/frag thresholds upon vdev creation
>>   ath10k: do not setup rts/frag thresholds for suspended interfaces
>> 
>>  drivers/net/wireless/ath/ath10k/mac.c |   30 +++++++++++++++++++++++++-----
>>  drivers/net/wireless/ath/ath10k/pci.c |    2 +-
>>  2 files changed, 26 insertions(+), 6 deletions(-)

Thanks, applied.

> Other than the observation about an incomplete FW feature,
> this looks good to me.

I applied that patch anyway as I'm hoping it will be implemented in fw
eventually. Thanks for the review.

-- 
Kalle Valo