qca6174 with wmi-tlv firmware uses offloaded
beaconing scheme (i.e. templates). This requires a
little different approach when implementing CSA.
Add missing code to update CS count and report CSA
completion to mac80211. Without it channel switch
was never finished.
To avoid races during interface teardown data_lock
has been used to protect is_up and is_started so
they can be compared against before scheduling
count down work.
Signed-off-by: Michal Kazior <[email protected]>
---
@Kalle: I'm deferring the chanctx patchset v4
until this gets applied. I'd like to avoid posting
patches without a valid parent id and leaving you
with some weird conflicts and implicit depndencies
in the code to deal with.
drivers/net/wireless/ath/ath10k/core.h | 1 +
drivers/net/wireless/ath/ath10k/mac.c | 70 ++++++++++++++++++++++++++++++-
drivers/net/wireless/ath/ath10k/wmi-tlv.c | 8 ++++
3 files changed, 77 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index d60e46f..43fa2ec 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -320,6 +320,7 @@ struct ath10k_vif {
int num_legacy_stations;
int txpower;
struct wmi_wmm_params_all_arg wmm_params;
+ struct work_struct ap_csa_work;
};
struct ath10k_vif_iter {
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index d6d2f0f..631e73b 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -1055,6 +1055,10 @@ static int ath10k_mac_setup_bcn_tmpl(struct ath10k_vif *arvif)
if (!test_bit(WMI_SERVICE_BEACON_OFFLOAD, ar->wmi.svc_map))
return 0;
+ if (arvif->vdev_type != WMI_VDEV_TYPE_AP &&
+ arvif->vdev_type != WMI_VDEV_TYPE_IBSS)
+ return 0;
+
bcn = ieee80211_beacon_get_template(hw, vif, &offs);
if (!bcn) {
ath10k_warn(ar, "failed to get beacon template from mac80211\n");
@@ -1100,6 +1104,9 @@ static int ath10k_mac_setup_prb_tmpl(struct ath10k_vif *arvif)
if (!test_bit(WMI_SERVICE_BEACON_OFFLOAD, ar->wmi.svc_map))
return 0;
+ if (arvif->vdev_type != WMI_VDEV_TYPE_AP)
+ return 0;
+
prb = ieee80211_proberesp_get(hw, vif);
if (!prb) {
ath10k_warn(ar, "failed to get probe resp template from mac80211\n");
@@ -1129,10 +1136,9 @@ static void ath10k_control_beaconing(struct ath10k_vif *arvif,
if (!info->enable_beacon) {
ath10k_vdev_stop(arvif);
+ spin_lock_bh(&arvif->ar->data_lock);
arvif->is_started = false;
arvif->is_up = false;
-
- spin_lock_bh(&arvif->ar->data_lock);
ath10k_mac_vif_beacon_free(arvif);
spin_unlock_bh(&arvif->ar->data_lock);
@@ -1335,6 +1341,49 @@ static int ath10k_mac_vif_disable_keepalive(struct ath10k_vif *arvif)
return 0;
}
+static void ath10k_mac_vif_ap_csa_count_down(struct ath10k_vif *arvif)
+{
+ struct ath10k *ar = arvif->ar;
+ struct ieee80211_vif *vif = arvif->vif;
+ int ret;
+
+ if (arvif->vdev_type != WMI_VDEV_TYPE_AP)
+ return;
+
+ if (!vif->csa_active)
+ return;
+
+ if (!arvif->is_up)
+ return;
+
+ if (!ieee80211_csa_is_complete(vif)) {
+ ieee80211_csa_update_counter(vif);
+
+ ret = ath10k_mac_setup_bcn_tmpl(arvif);
+ if (ret)
+ ath10k_warn(ar, "failed to update bcn tmpl during csa: %d\n",
+ ret);
+
+ ret = ath10k_mac_setup_prb_tmpl(arvif);
+ if (ret)
+ ath10k_warn(ar, "failed to update prb tmpl during csa: %d\n",
+ ret);
+ } else {
+ ieee80211_csa_finish(vif);
+ }
+}
+
+static void ath10k_mac_vif_ap_csa_work(struct work_struct *work)
+{
+ struct ath10k_vif *arvif = container_of(work, struct ath10k_vif,
+ ap_csa_work);
+ struct ath10k *ar = arvif->ar;
+
+ mutex_lock(&ar->conf_mutex);
+ ath10k_mac_vif_ap_csa_count_down(arvif);
+ mutex_unlock(&ar->conf_mutex);
+}
+
/**********************/
/* Station management */
/**********************/
@@ -1852,7 +1901,9 @@ static void ath10k_bss_assoc(struct ieee80211_hw *hw,
return;
}
+ spin_lock_bh(&arvif->ar->data_lock);
arvif->is_up = true;
+ spin_unlock_bh(&arvif->ar->data_lock);
}
static void ath10k_bss_disassoc(struct ieee80211_hw *hw,
@@ -1874,7 +1925,9 @@ static void ath10k_bss_disassoc(struct ieee80211_hw *hw,
arvif->def_wep_key_idx = -1;
+ spin_lock_bh(&arvif->ar->data_lock);
arvif->is_up = false;
+ spin_unlock_bh(&arvif->ar->data_lock);
}
static int ath10k_station_assoc(struct ath10k *ar,
@@ -2931,6 +2984,16 @@ static void ath10k_config_chan(struct ath10k *ar)
if (arvif->vdev_type == WMI_VDEV_TYPE_MONITOR)
continue;
+ ret = ath10k_mac_setup_bcn_tmpl(arvif);
+ if (ret)
+ ath10k_warn(ar, "failed to update bcn tmpl during csa: %d\n",
+ ret);
+
+ ret = ath10k_mac_setup_prb_tmpl(arvif);
+ if (ret)
+ ath10k_warn(ar, "failed to update prb tmpl during csa: %d\n",
+ ret);
+
ret = ath10k_vdev_restart(arvif);
if (ret) {
ath10k_warn(ar, "failed to restart vdev %d: %d\n",
@@ -3091,6 +3154,7 @@ static int ath10k_add_interface(struct ieee80211_hw *hw,
arvif->vif = vif;
INIT_LIST_HEAD(&arvif->list);
+ INIT_WORK(&arvif->ap_csa_work, ath10k_mac_vif_ap_csa_work);
if (ar->free_vdev_map == 0) {
ath10k_warn(ar, "Free vdev map is empty, no more interfaces allowed.\n");
@@ -3308,6 +3372,8 @@ static void ath10k_remove_interface(struct ieee80211_hw *hw,
struct ath10k_vif *arvif = ath10k_vif_to_arvif(vif);
int ret;
+ cancel_work_sync(&arvif->ap_csa_work);
+
mutex_lock(&ar->conf_mutex);
spin_lock_bh(&ar->data_lock);
diff --git a/drivers/net/wireless/ath/ath10k/wmi-tlv.c b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
index 71614ba..538a643 100644
--- a/drivers/net/wireless/ath/ath10k/wmi-tlv.c
+++ b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
@@ -17,6 +17,7 @@
#include "core.h"
#include "debug.h"
#include "hw.h"
+#include "mac.h"
#include "wmi.h"
#include "wmi-ops.h"
#include "wmi-tlv.h"
@@ -168,6 +169,7 @@ static int ath10k_wmi_tlv_event_bcn_tx_status(struct ath10k *ar,
{
const void **tb;
const struct wmi_tlv_bcn_tx_status_ev *ev;
+ struct ath10k_vif *arvif;
u32 vdev_id, tx_status;
int ret;
@@ -201,6 +203,12 @@ static int ath10k_wmi_tlv_event_bcn_tx_status(struct ath10k *ar,
break;
}
+ spin_lock_bh(&ar->data_lock);
+ arvif = ath10k_get_arvif(ar, vdev_id);
+ if (arvif && arvif->is_up)
+ ieee80211_queue_work(ar->hw, &arvif->ap_csa_work);
+ spin_unlock_bh(&ar->data_lock);
+
kfree(tb);
return 0;
}
--
1.8.5.3
Michal Kazior <[email protected]> writes:
>> I actually had strange conflicts already with this patch (in mac.c). As
>> I'm travelling, please check my resolution from the pending branch very
>> carefully. I might have done something stupid.
>
> There's a mistake. Only `arvif->is_up` should be protected by
> `ar->data_lock` in ath10k_bss_assoc():
>
> @@ -1852,7 +1901,9 @@ static void ath10k_bss_assoc(struct ieee80211_hw *hw,
> return;
> }
>
> + spin_lock_bh(&arvif->ar->data_lock);
> arvif->is_up = true;
> + spin_unlock_bh(&arvif->ar->data_lock);
>
> /* Workaround: Some firmware revisions (tested with qca6174
Thanks for catching this. I fixed it now, can you please check again?
--
Kalle Valo
On 15 February 2015 at 16:51, Kalle Valo <[email protected]> wrote:
> Michal Kazior <[email protected]> writes:
>
>> qca6174 with wmi-tlv firmware uses offloaded
>> beaconing scheme (i.e. templates). This requires a
>> little different approach when implementing CSA.
>>
>> Add missing code to update CS count and report CSA
>> completion to mac80211. Without it channel switch
>> was never finished.
>>
>> To avoid races during interface teardown data_lock
>> has been used to protect is_up and is_started so
>> they can be compared against before scheduling
>> count down work.
>>
>> Signed-off-by: Michal Kazior <[email protected]>
>> ---
>>
>> @Kalle: I'm deferring the chanctx patchset v4
>> until this gets applied. I'd like to avoid posting
>> patches without a valid parent id and leaving you
>> with some weird conflicts and implicit depndencies
>> in the code to deal with.
>
> Ok.
>
> I actually had strange conflicts already with this patch (in mac.c). As
> I'm travelling, please check my resolution from the pending branch very
> carefully. I might have done something stupid.
There's a mistake. Only `arvif->is_up` should be protected by
`ar->data_lock` in ath10k_bss_assoc():
@@ -1852,7 +1901,9 @@ static void ath10k_bss_assoc(struct ieee80211_hw *hw,
return;
}
+ spin_lock_bh(&arvif->ar->data_lock);
arvif->is_up = true;
+ spin_unlock_bh(&arvif->ar->data_lock);
/* Workaround: Some firmware revisions (tested with qca6174
(...)
}
Michał
qca6174 with wmi-tlv firmware uses offloaded
beaconing scheme (i.e. templates). This requires a
little different approach when implementing CSA.
Add missing code to update CS count and report CSA
completion to mac80211. Without it channel switch
was never finished.
Signed-off-by: Michal Kazior <[email protected]>
---
Notes:
v2:
* rebased
* remove data_lock usage (it wasn't necessary after all)
* add lockdep
* add sanity WARN_ON
* check for csa_active before queueing ap csa work too
drivers/net/wireless/ath/ath10k/core.h | 1 +
drivers/net/wireless/ath/ath10k/mac.c | 68 +++++++++++++++++++++++++++++++
drivers/net/wireless/ath/ath10k/wmi-tlv.c | 6 +++
3 files changed, 75 insertions(+)
diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index 7cba781..726c6e5 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -340,6 +340,7 @@ struct ath10k_vif {
int num_legacy_stations;
int txpower;
struct wmi_wmm_params_all_arg wmm_params;
+ struct work_struct ap_csa_work;
};
struct ath10k_vif_iter {
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 0f39af7..5440596 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -1056,6 +1056,10 @@ static int ath10k_mac_setup_bcn_tmpl(struct ath10k_vif *arvif)
if (!test_bit(WMI_SERVICE_BEACON_OFFLOAD, ar->wmi.svc_map))
return 0;
+ if (arvif->vdev_type != WMI_VDEV_TYPE_AP &&
+ arvif->vdev_type != WMI_VDEV_TYPE_IBSS)
+ return 0;
+
bcn = ieee80211_beacon_get_template(hw, vif, &offs);
if (!bcn) {
ath10k_warn(ar, "failed to get beacon template from mac80211\n");
@@ -1101,6 +1105,9 @@ static int ath10k_mac_setup_prb_tmpl(struct ath10k_vif *arvif)
if (!test_bit(WMI_SERVICE_BEACON_OFFLOAD, ar->wmi.svc_map))
return 0;
+ if (arvif->vdev_type != WMI_VDEV_TYPE_AP)
+ return 0;
+
prb = ieee80211_proberesp_get(hw, vif);
if (!prb) {
ath10k_warn(ar, "failed to get probe resp template from mac80211\n");
@@ -1336,6 +1343,54 @@ static int ath10k_mac_vif_disable_keepalive(struct ath10k_vif *arvif)
return 0;
}
+static void ath10k_mac_vif_ap_csa_count_down(struct ath10k_vif *arvif)
+{
+ struct ath10k *ar = arvif->ar;
+ struct ieee80211_vif *vif = arvif->vif;
+ int ret;
+
+ lockdep_assert_held(&arvif->ar->conf_mutex);
+
+ if (WARN_ON(!test_bit(WMI_SERVICE_BEACON_OFFLOAD, ar->wmi.svc_map)))
+ return;
+
+ if (arvif->vdev_type != WMI_VDEV_TYPE_AP)
+ return;
+
+ if (!vif->csa_active)
+ return;
+
+ if (!arvif->is_up)
+ return;
+
+ if (!ieee80211_csa_is_complete(vif)) {
+ ieee80211_csa_update_counter(vif);
+
+ ret = ath10k_mac_setup_bcn_tmpl(arvif);
+ if (ret)
+ ath10k_warn(ar, "failed to update bcn tmpl during csa: %d\n",
+ ret);
+
+ ret = ath10k_mac_setup_prb_tmpl(arvif);
+ if (ret)
+ ath10k_warn(ar, "failed to update prb tmpl during csa: %d\n",
+ ret);
+ } else {
+ ieee80211_csa_finish(vif);
+ }
+}
+
+static void ath10k_mac_vif_ap_csa_work(struct work_struct *work)
+{
+ struct ath10k_vif *arvif = container_of(work, struct ath10k_vif,
+ ap_csa_work);
+ struct ath10k *ar = arvif->ar;
+
+ mutex_lock(&ar->conf_mutex);
+ ath10k_mac_vif_ap_csa_count_down(arvif);
+ mutex_unlock(&ar->conf_mutex);
+}
+
/**********************/
/* Station management */
/**********************/
@@ -3022,6 +3077,16 @@ static void ath10k_config_chan(struct ath10k *ar)
if (arvif->vdev_type == WMI_VDEV_TYPE_MONITOR)
continue;
+ ret = ath10k_mac_setup_bcn_tmpl(arvif);
+ if (ret)
+ ath10k_warn(ar, "failed to update bcn tmpl during csa: %d\n",
+ ret);
+
+ ret = ath10k_mac_setup_prb_tmpl(arvif);
+ if (ret)
+ ath10k_warn(ar, "failed to update prb tmpl during csa: %d\n",
+ ret);
+
ret = ath10k_vdev_restart(arvif);
if (ret) {
ath10k_warn(ar, "failed to restart vdev %d: %d\n",
@@ -3182,6 +3247,7 @@ static int ath10k_add_interface(struct ieee80211_hw *hw,
arvif->vif = vif;
INIT_LIST_HEAD(&arvif->list);
+ INIT_WORK(&arvif->ap_csa_work, ath10k_mac_vif_ap_csa_work);
if (ar->free_vdev_map == 0) {
ath10k_warn(ar, "Free vdev map is empty, no more interfaces allowed.\n");
@@ -3399,6 +3465,8 @@ static void ath10k_remove_interface(struct ieee80211_hw *hw,
struct ath10k_vif *arvif = ath10k_vif_to_arvif(vif);
int ret;
+ cancel_work_sync(&arvif->ap_csa_work);
+
mutex_lock(&ar->conf_mutex);
spin_lock_bh(&ar->data_lock);
diff --git a/drivers/net/wireless/ath/ath10k/wmi-tlv.c b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
index f34baa0..94c4425 100644
--- a/drivers/net/wireless/ath/ath10k/wmi-tlv.c
+++ b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
@@ -17,6 +17,7 @@
#include "core.h"
#include "debug.h"
#include "hw.h"
+#include "mac.h"
#include "wmi.h"
#include "wmi-ops.h"
#include "wmi-tlv.h"
@@ -168,6 +169,7 @@ static int ath10k_wmi_tlv_event_bcn_tx_status(struct ath10k *ar,
{
const void **tb;
const struct wmi_tlv_bcn_tx_status_ev *ev;
+ struct ath10k_vif *arvif;
u32 vdev_id, tx_status;
int ret;
@@ -201,6 +203,10 @@ static int ath10k_wmi_tlv_event_bcn_tx_status(struct ath10k *ar,
break;
}
+ arvif = ath10k_get_arvif(ar, vdev_id);
+ if (arvif && arvif->is_up && arvif->vif->csa_active)
+ ieee80211_queue_work(ar->hw, &arvif->ap_csa_work);
+
kfree(tb);
return 0;
}
--
1.8.5.3
Michal Kazior <[email protected]> writes:
> qca6174 with wmi-tlv firmware uses offloaded
> beaconing scheme (i.e. templates). This requires a
> little different approach when implementing CSA.
>
> Add missing code to update CS count and report CSA
> completion to mac80211. Without it channel switch
> was never finished.
>
> To avoid races during interface teardown data_lock
> has been used to protect is_up and is_started so
> they can be compared against before scheduling
> count down work.
>
> Signed-off-by: Michal Kazior <[email protected]>
> ---
>
> @Kalle: I'm deferring the chanctx patchset v4
> until this gets applied. I'd like to avoid posting
> patches without a valid parent id and leaving you
> with some weird conflicts and implicit depndencies
> in the code to deal with.
Ok.
I actually had strange conflicts already with this patch (in mac.c). As
I'm travelling, please check my resolution from the pending branch very
carefully. I might have done something stupid.
--
Kalle Valo
Michal Kazior <[email protected]> writes:
> On 7 March 2015 at 08:46, Kalle Valo <[email protected]> wrote:
>> Michal Kazior <[email protected]> writes:
>>
>>> qca6174 with wmi-tlv firmware uses offloaded
>>> beaconing scheme (i.e. templates). This requires a
>>> little different approach when implementing CSA.
>>>
>>> Add missing code to update CS count and report CSA
>>> completion to mac80211. Without it channel switch
>>> was never finished.
>>>
>>> Signed-off-by: Michal Kazior <[email protected]>
>>
>> Thanks, applied.
>
> It seems you've merged v1 instead of v2.. I'll send a follow up fix later.
Oh, sorry about that. Please do send a followup patch.
--
Kalle Valo
On 7 March 2015 at 08:46, Kalle Valo <[email protected]> wrote:
> Michal Kazior <[email protected]> writes:
>
>> qca6174 with wmi-tlv firmware uses offloaded
>> beaconing scheme (i.e. templates). This requires a
>> little different approach when implementing CSA.
>>
>> Add missing code to update CS count and report CSA
>> completion to mac80211. Without it channel switch
>> was never finished.
>>
>> Signed-off-by: Michal Kazior <[email protected]>
>
> Thanks, applied.
It seems you've merged v1 instead of v2.. I'll send a follow up fix later.
Michał
Mistakenly v1 of `ath10k: fix AP/IBSS CSA with
template based fw`
(81a9a17db5d766d2872c4ab8a77b8666d22031ec) was
applied.
This patch applies the missing bits from v2:
* remove unnecessary locking
* add some (sanity) checks
There are no practical functionality differences
between v1 and v2.
Signed-off-by: Michal Kazior <[email protected]>
---
drivers/net/wireless/ath/ath10k/mac.c | 12 +++++++-----
drivers/net/wireless/ath/ath10k/wmi-tlv.c | 4 +---
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 1138f4f..2f9518a 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -1230,9 +1230,10 @@ static void ath10k_control_beaconing(struct ath10k_vif *arvif,
if (!info->enable_beacon) {
ath10k_vdev_stop(arvif);
- spin_lock_bh(&arvif->ar->data_lock);
arvif->is_started = false;
arvif->is_up = false;
+
+ spin_lock_bh(&arvif->ar->data_lock);
ath10k_mac_vif_beacon_free(arvif);
spin_unlock_bh(&arvif->ar->data_lock);
@@ -1466,6 +1467,11 @@ static void ath10k_mac_vif_ap_csa_count_down(struct ath10k_vif *arvif)
struct ieee80211_vif *vif = arvif->vif;
int ret;
+ lockdep_assert_held(&arvif->ar->conf_mutex);
+
+ if (WARN_ON(!test_bit(WMI_SERVICE_BEACON_OFFLOAD, ar->wmi.svc_map)))
+ return;
+
if (arvif->vdev_type != WMI_VDEV_TYPE_AP)
return;
@@ -2092,9 +2098,7 @@ static void ath10k_bss_assoc(struct ieee80211_hw *hw,
return;
}
- spin_lock_bh(&arvif->ar->data_lock);
arvif->is_up = true;
- spin_unlock_bh(&arvif->ar->data_lock);
/* Workaround: Some firmware revisions (tested with qca6174
* WLAN.RM.2.0-00073) have buggy powersave state machine and must be
@@ -2136,9 +2140,7 @@ static void ath10k_bss_disassoc(struct ieee80211_hw *hw,
return;
}
- spin_lock_bh(&arvif->ar->data_lock);
arvif->is_up = false;
- spin_unlock_bh(&arvif->ar->data_lock);
}
static int ath10k_station_assoc(struct ath10k *ar,
diff --git a/drivers/net/wireless/ath/ath10k/wmi-tlv.c b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
index 939d86b..2e12637 100644
--- a/drivers/net/wireless/ath/ath10k/wmi-tlv.c
+++ b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
@@ -206,11 +206,9 @@ static int ath10k_wmi_tlv_event_bcn_tx_status(struct ath10k *ar,
break;
}
- spin_lock_bh(&ar->data_lock);
arvif = ath10k_get_arvif(ar, vdev_id);
- if (arvif && arvif->is_up)
+ if (arvif && arvif->is_up && arvif->vif->csa_active)
ieee80211_queue_work(ar->hw, &arvif->ap_csa_work);
- spin_unlock_bh(&ar->data_lock);
kfree(tb);
return 0;
--
1.8.5.3
Michal Kazior <[email protected]> writes:
> qca6174 with wmi-tlv firmware uses offloaded
> beaconing scheme (i.e. templates). This requires a
> little different approach when implementing CSA.
>
> Add missing code to update CS count and report CSA
> completion to mac80211. Without it channel switch
> was never finished.
>
> Signed-off-by: Michal Kazior <[email protected]>
Thanks, applied.
--
Kalle Valo
Michal Kazior <[email protected]> writes:
> Mistakenly v1 of `ath10k: fix AP/IBSS CSA with
> template based fw`
> (81a9a17db5d766d2872c4ab8a77b8666d22031ec) was
> applied.
>
> This patch applies the missing bits from v2:
> * remove unnecessary locking
> * add some (sanity) checks
>
> There are no practical functionality differences
> between v1 and v2.
>
> Signed-off-by: Michal Kazior <[email protected]>
Thanks, applied. Sorry for my mistake and kudos for catching it so fast!
--
Kalle Valo