Hi,
This has some fixes I've stacked in my local repo.
There are no dependencies between them.
Michal Kazior (3):
ath10k: use nss provided by mac80211
ath10k: implement sta_rc_update()
ath10k: fix Tx status clearing
drivers/net/wireless/ath/ath10k/mac.c | 88 +++++++++++++++++++++++++++++++++-
drivers/net/wireless/ath/ath10k/txrx.c | 2 +-
drivers/net/wireless/ath/ath10k/wmi.h | 6 +++
3 files changed, 93 insertions(+), 3 deletions(-)
--
1.8.4.rc3
Calculating STA NSS just from the mcs rateset is
not the greatest idea.
This should prevent connectivity issues if
mac80211 is ever to set rx_nss to something other
rather than base on max mcs map. As an example
operation mode change notification in assoc
request may change rx_nss initial values in the
future.
Signed-off-by: Michal Kazior <[email protected]>
---
drivers/net/wireless/ath/ath10k/mac.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index b70a3b2..15eda44 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -925,7 +925,7 @@ static void ath10k_peer_assoc_h_basic(struct ath10k *ar,
else
arg->peer_listen_intval = ar->hw->conf.listen_interval;
- arg->peer_num_spatial_streams = 1;
+ arg->peer_num_spatial_streams = max_t(u32, 1, sta->rx_nss);
/*
* The assoc capabilities are available only in managed mode.
@@ -1075,7 +1075,6 @@ static void ath10k_peer_assoc_h_ht(struct ath10k *ar,
arg->peer_ht_rates.rates[n++] = i;
arg->peer_ht_rates.num_rates = n;
- arg->peer_num_spatial_streams = max((n+7) / 8, 1);
ath10k_dbg(ATH10K_DBG_MAC, "mac ht peer %pM mcs cnt %d nss %d\n",
arg->addr,
--
1.8.4.rc3
Too much of tx info was being cleared. This caused
issues in some setups with tx frame status
reporting.
This should fix some cases of stations not being
able to associate to ath10k AP.
Reported-By: Matti Laakso <[email protected]>
Signed-off-by: Michal Kazior <[email protected]>
---
drivers/net/wireless/ath/ath10k/txrx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/ath/ath10k/txrx.c b/drivers/net/wireless/ath/ath10k/txrx.c
index d476b2c..2282980 100644
--- a/drivers/net/wireless/ath/ath10k/txrx.c
+++ b/drivers/net/wireless/ath/ath10k/txrx.c
@@ -75,7 +75,7 @@ void ath10k_txrx_tx_unref(struct ath10k_htt *htt,
ath10k_report_offchan_tx(htt->ar, msdu);
info = IEEE80211_SKB_CB(msdu);
- memset(info, 0, sizeof(*info));
+ memset(&info->status, 0, sizeof(info->status));
if (tx_done->discard) {
ieee80211_free_txskb(htt->ar->hw, msdu);
--
1.8.4.rc3
Hi,
This has some fixes I've stacked in my local repo.
There are no dependencies between them.
v2:
* refined commit messages
Michal Kazior (3):
ath10k: use nss provided by mac80211
ath10k: implement sta_rc_update()
ath10k: fix Tx status clearing
drivers/net/wireless/ath/ath10k/mac.c | 88 +++++++++++++++++++++++++++++++++-
drivers/net/wireless/ath/ath10k/txrx.c | 2 +-
drivers/net/wireless/ath/ath10k/wmi.h | 6 +++
3 files changed, 93 insertions(+), 3 deletions(-)
--
1.8.4.rc3
This should fix possible connectivity issues upon
changes of channel width, number of streams or
SMPS on connected stations.
An example trigger would be an action frame with
operation mode change notification.
Signed-off-by: Michal Kazior <[email protected]>
---
drivers/net/wireless/ath/ath10k/mac.c | 85 +++++++++++++++++++++++++++++++++++
drivers/net/wireless/ath/ath10k/wmi.h | 6 +++
2 files changed, 91 insertions(+)
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 15eda44..65cb63d 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -3269,6 +3269,90 @@ exit:
return ret;
}
+static void ath10k_sta_rc_update(struct ieee80211_hw *hw,
+ struct ieee80211_vif *vif,
+ struct ieee80211_sta *sta,
+ u32 changed)
+{
+ struct ath10k *ar = hw->priv;
+ struct ath10k_vif *arvif = ath10k_vif_to_arvif(vif);
+ u32 chwidth, smps;
+ int ret;
+
+ if (changed & IEEE80211_RC_BW_CHANGED) {
+ switch (sta->bandwidth) {
+ default:
+ ath10k_warn("unsupported STA BW: %d\n", sta->bandwidth);
+ case IEEE80211_STA_RX_BW_20:
+ chwidth = WMI_PEER_CHWIDTH_20MHZ;
+ break;
+ case IEEE80211_STA_RX_BW_40:
+ chwidth = WMI_PEER_CHWIDTH_40MHZ;
+ break;
+ case IEEE80211_STA_RX_BW_80:
+ chwidth = WMI_PEER_CHWIDTH_80MHZ;
+ break;
+ }
+
+ ath10k_dbg(ATH10K_DBG_MAC,
+ "mac update sta %pM bandwidth (peer chwidth %d) to %d\n",
+ sta->addr, chwidth, sta->bandwidth);
+
+ ret = ath10k_wmi_peer_set_param(ar, arvif->vdev_id, sta->addr,
+ WMI_PEER_CHAN_WIDTH, chwidth);
+ if (ret)
+ ath10k_warn("failed to update STA %pM bandwidth (peer chwidth %d) to %d: %d\n",
+ sta->addr, chwidth, sta->bandwidth, ret);
+ }
+
+ if (changed & IEEE80211_RC_NSS_CHANGED) {
+ ath10k_dbg(ATH10K_DBG_MAC, "mac update sta %pM nss to %d\n",
+ sta->addr, sta->rx_nss);
+
+ ret = ath10k_wmi_peer_set_param(ar, arvif->vdev_id, sta->addr,
+ WMI_PEER_NSS, sta->rx_nss);
+ if (ret)
+ ath10k_warn("failed to update STA %pM nss to %d: %d\n",
+ sta->addr, sta->rx_nss, ret);
+ }
+
+ if (changed & IEEE80211_RC_SMPS_CHANGED) {
+ smps = WMI_PEER_SMPS_PS_NONE;
+
+ switch (sta->smps_mode) {
+ case IEEE80211_SMPS_NUM_MODES:
+ ath10k_warn("invalid smps mode: %d\n", sta->smps_mode);
+ case IEEE80211_SMPS_AUTOMATIC:
+ case IEEE80211_SMPS_OFF:
+ smps = WMI_PEER_SMPS_PS_NONE;
+ break;
+ case IEEE80211_SMPS_STATIC:
+ smps = WMI_PEER_SMPS_STATIC;
+ break;
+ case IEEE80211_SMPS_DYNAMIC:
+ smps = WMI_PEER_SMPS_DYNAMIC;
+ break;
+ }
+
+ ath10k_dbg(ATH10K_DBG_MAC,
+ "mac update sta %pM smps (peer smps %d) to %d\n",
+ sta->addr, smps, sta->smps_mode);
+
+ ret = ath10k_wmi_peer_set_param(ar, arvif->vdev_id, sta->addr,
+ WMI_PEER_SMPS_STATE, smps);
+ if (ret)
+ ath10k_warn("failed to update STA %pM smps (peer smps %d) to %d: %d\n",
+ sta->addr, smps, sta->smps_mode, ret);
+ }
+
+ if (changed & IEEE80211_RC_SUPP_RATES_CHANGED) {
+ /* FIXME: Not implemented. Not really sure if it's possible to
+ * influence HW rate control so much. */
+ ath10k_dbg(ATH10K_DBG_MAC,
+ "mac sta rc update - supp rates changed - not implemented\n");
+ }
+}
+
static const struct ieee80211_ops ath10k_ops = {
.tx = ath10k_tx,
.start = ath10k_start,
@@ -3291,6 +3375,7 @@ static const struct ieee80211_ops ath10k_ops = {
.tx_last_beacon = ath10k_tx_last_beacon,
.restart_complete = ath10k_restart_complete,
.get_survey = ath10k_get_survey,
+ .sta_rc_update = ath10k_sta_rc_update,
#ifdef CONFIG_PM
.suspend = ath10k_suspend,
.resume = ath10k_resume,
diff --git a/drivers/net/wireless/ath/ath10k/wmi.h b/drivers/net/wireless/ath/ath10k/wmi.h
index 0087d69..e8c4bb7 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.h
+++ b/drivers/net/wireless/ath/ath10k/wmi.h
@@ -3847,6 +3847,12 @@ enum wmi_peer_smps_state {
WMI_PEER_SMPS_DYNAMIC = 0x2
};
+enum wmi_peer_chwidth {
+ WMI_PEER_CHWIDTH_20MHZ = 0,
+ WMI_PEER_CHWIDTH_40MHZ = 1,
+ WMI_PEER_CHWIDTH_80MHZ = 2,
+};
+
enum wmi_peer_param {
WMI_PEER_SMPS_STATE = 0x1, /* see %wmi_peer_smps_state */
WMI_PEER_AMPDU = 0x2,
--
1.8.4.rc3
On Wed, 2013-11-27 at 11:25 +0100, Michal Kazior wrote:
> On 26 November 2013 14:57, Michal Kazior <[email protected]> wrote:
> > Calculating STA NSS just from the mcs rateset is
> > not the greatest idea.
> >
> > This should prevent connectivity issues if
> > mac80211 is ever to set rx_nss to something other
> > rather than base on max mcs map. As an example
> > operation mode change notification in assoc
> > request may change rx_nss initial values in the
> > future.
> >
> > Signed-off-by: Michal Kazior <[email protected]>
> > ---
>
> Drop this single patch, please.
>
> I just noticed rx_nss doesn't seem to have correct value when associating..
FWIW, there are pending changes to make the opmode data come from
hostapd to mac80211 so that this would be set correctly when the station
is associating.
Or where did you find it wasn't set correctly?
johannes
Michal Kazior <[email protected]> writes:
> Too much of tx info was being cleared. This caused
> issues in some setups.
>
> Reported-By: Matti Laakso <[email protected]>
> Signed-off-by: Michal Kazior <[email protected]>
Please describe the issues (or at least some of them) in the commit log.
The idea is that an advanced ath10k user will understand what kind of
bug (from user's point of view) this patch is fixing.
--
Kalle Valo
This allows dynamic changes of bandwidth/nss, e.g.
via ht/vht operation mode change notification.
Signed-off-by: Michal Kazior <[email protected]>
---
drivers/net/wireless/ath/ath10k/mac.c | 85 +++++++++++++++++++++++++++++++++++
drivers/net/wireless/ath/ath10k/wmi.h | 6 +++
2 files changed, 91 insertions(+)
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 15eda44..65cb63d 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -3269,6 +3269,90 @@ exit:
return ret;
}
+static void ath10k_sta_rc_update(struct ieee80211_hw *hw,
+ struct ieee80211_vif *vif,
+ struct ieee80211_sta *sta,
+ u32 changed)
+{
+ struct ath10k *ar = hw->priv;
+ struct ath10k_vif *arvif = ath10k_vif_to_arvif(vif);
+ u32 chwidth, smps;
+ int ret;
+
+ if (changed & IEEE80211_RC_BW_CHANGED) {
+ switch (sta->bandwidth) {
+ default:
+ ath10k_warn("unsupported STA BW: %d\n", sta->bandwidth);
+ case IEEE80211_STA_RX_BW_20:
+ chwidth = WMI_PEER_CHWIDTH_20MHZ;
+ break;
+ case IEEE80211_STA_RX_BW_40:
+ chwidth = WMI_PEER_CHWIDTH_40MHZ;
+ break;
+ case IEEE80211_STA_RX_BW_80:
+ chwidth = WMI_PEER_CHWIDTH_80MHZ;
+ break;
+ }
+
+ ath10k_dbg(ATH10K_DBG_MAC,
+ "mac update sta %pM bandwidth (peer chwidth %d) to %d\n",
+ sta->addr, chwidth, sta->bandwidth);
+
+ ret = ath10k_wmi_peer_set_param(ar, arvif->vdev_id, sta->addr,
+ WMI_PEER_CHAN_WIDTH, chwidth);
+ if (ret)
+ ath10k_warn("failed to update STA %pM bandwidth (peer chwidth %d) to %d: %d\n",
+ sta->addr, chwidth, sta->bandwidth, ret);
+ }
+
+ if (changed & IEEE80211_RC_NSS_CHANGED) {
+ ath10k_dbg(ATH10K_DBG_MAC, "mac update sta %pM nss to %d\n",
+ sta->addr, sta->rx_nss);
+
+ ret = ath10k_wmi_peer_set_param(ar, arvif->vdev_id, sta->addr,
+ WMI_PEER_NSS, sta->rx_nss);
+ if (ret)
+ ath10k_warn("failed to update STA %pM nss to %d: %d\n",
+ sta->addr, sta->rx_nss, ret);
+ }
+
+ if (changed & IEEE80211_RC_SMPS_CHANGED) {
+ smps = WMI_PEER_SMPS_PS_NONE;
+
+ switch (sta->smps_mode) {
+ case IEEE80211_SMPS_NUM_MODES:
+ ath10k_warn("invalid smps mode: %d\n", sta->smps_mode);
+ case IEEE80211_SMPS_AUTOMATIC:
+ case IEEE80211_SMPS_OFF:
+ smps = WMI_PEER_SMPS_PS_NONE;
+ break;
+ case IEEE80211_SMPS_STATIC:
+ smps = WMI_PEER_SMPS_STATIC;
+ break;
+ case IEEE80211_SMPS_DYNAMIC:
+ smps = WMI_PEER_SMPS_DYNAMIC;
+ break;
+ }
+
+ ath10k_dbg(ATH10K_DBG_MAC,
+ "mac update sta %pM smps (peer smps %d) to %d\n",
+ sta->addr, smps, sta->smps_mode);
+
+ ret = ath10k_wmi_peer_set_param(ar, arvif->vdev_id, sta->addr,
+ WMI_PEER_SMPS_STATE, smps);
+ if (ret)
+ ath10k_warn("failed to update STA %pM smps (peer smps %d) to %d: %d\n",
+ sta->addr, smps, sta->smps_mode, ret);
+ }
+
+ if (changed & IEEE80211_RC_SUPP_RATES_CHANGED) {
+ /* FIXME: Not implemented. Not really sure if it's possible to
+ * influence HW rate control so much. */
+ ath10k_dbg(ATH10K_DBG_MAC,
+ "mac sta rc update - supp rates changed - not implemented\n");
+ }
+}
+
static const struct ieee80211_ops ath10k_ops = {
.tx = ath10k_tx,
.start = ath10k_start,
@@ -3291,6 +3375,7 @@ static const struct ieee80211_ops ath10k_ops = {
.tx_last_beacon = ath10k_tx_last_beacon,
.restart_complete = ath10k_restart_complete,
.get_survey = ath10k_get_survey,
+ .sta_rc_update = ath10k_sta_rc_update,
#ifdef CONFIG_PM
.suspend = ath10k_suspend,
.resume = ath10k_resume,
diff --git a/drivers/net/wireless/ath/ath10k/wmi.h b/drivers/net/wireless/ath/ath10k/wmi.h
index 0087d69..e8c4bb7 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.h
+++ b/drivers/net/wireless/ath/ath10k/wmi.h
@@ -3847,6 +3847,12 @@ enum wmi_peer_smps_state {
WMI_PEER_SMPS_DYNAMIC = 0x2
};
+enum wmi_peer_chwidth {
+ WMI_PEER_CHWIDTH_20MHZ = 0,
+ WMI_PEER_CHWIDTH_40MHZ = 1,
+ WMI_PEER_CHWIDTH_80MHZ = 2,
+};
+
enum wmi_peer_param {
WMI_PEER_SMPS_STATE = 0x1, /* see %wmi_peer_smps_state */
WMI_PEER_AMPDU = 0x2,
--
1.8.4.rc3
Michal Kazior <[email protected]> writes:
> Calculating STA NSS just from the rateset is not
> the greatest idea. The NSS could be possibly
> influenced by other things that only mac80211
> knows about.
>
> Signed-off-by: Michal Kazior <[email protected]>
Is there a bug which this patch fixes or what motivated you to do this?
--
Kalle Valo
Michal Kazior <[email protected]> writes:
> Hi,
>
> This has some fixes I've stacked in my local repo.
> There are no dependencies between them.
>
> v2:
> * refined commit messages
>
> Michal Kazior (3):
> ath10k: use nss provided by mac80211
Dropped by your request.
> ath10k: implement sta_rc_update()
I had comments for this.
> ath10k: fix Tx status clearing
This is applied.
--
Kalle Valo
Too much of tx info was being cleared. This caused
issues in some setups.
Reported-By: Matti Laakso <[email protected]>
Signed-off-by: Michal Kazior <[email protected]>
---
drivers/net/wireless/ath/ath10k/txrx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/ath/ath10k/txrx.c b/drivers/net/wireless/ath/ath10k/txrx.c
index d476b2c..2282980 100644
--- a/drivers/net/wireless/ath/ath10k/txrx.c
+++ b/drivers/net/wireless/ath/ath10k/txrx.c
@@ -75,7 +75,7 @@ void ath10k_txrx_tx_unref(struct ath10k_htt *htt,
ath10k_report_offchan_tx(htt->ar, msdu);
info = IEEE80211_SKB_CB(msdu);
- memset(info, 0, sizeof(*info));
+ memset(&info->status, 0, sizeof(info->status));
if (tx_done->discard) {
ieee80211_free_txskb(htt->ar->hw, msdu);
--
1.8.4.rc3
On 24 November 2013 14:50, Kalle Valo <[email protected]> wrote:
> Michal Kazior <[email protected]> writes:
>
>> Calculating STA NSS just from the rateset is not
>> the greatest idea. The NSS could be possibly
>> influenced by other things that only mac80211
>> knows about.
>>
>> Signed-off-by: Michal Kazior <[email protected]>
>
> Is there a bug which this patch fixes or what motivated you to do this?
This doesn't fix any bug that I know of. It just seems like probably
should use rx_nss instead of calculating it on our own (in case
something changes in the future).
A station may actually include operation mode change notification in
assoc request declaring it wants to use a different number of streams
than it actually supports as per its mcs map. That's not supported in
mac80211 though yet. Marek has posted some patches a few days ago but
it actually uses a different codepath from what I understand.
Michał
On 26 November 2013 14:57, Michal Kazior <[email protected]> wrote:
> Calculating STA NSS just from the mcs rateset is
> not the greatest idea.
>
> This should prevent connectivity issues if
> mac80211 is ever to set rx_nss to something other
> rather than base on max mcs map. As an example
> operation mode change notification in assoc
> request may change rx_nss initial values in the
> future.
>
> Signed-off-by: Michal Kazior <[email protected]>
> ---
Drop this single patch, please.
I just noticed rx_nss doesn't seem to have correct value when associating..
Michał
Michal Kazior <[email protected]> writes:
> On 26 November 2013 14:57, Michal Kazior <[email protected]> wrote:
>> Calculating STA NSS just from the mcs rateset is
>> not the greatest idea.
>>
>> This should prevent connectivity issues if
>> mac80211 is ever to set rx_nss to something other
>> rather than base on max mcs map. As an example
>> operation mode change notification in assoc
>> request may change rx_nss initial values in the
>> future.
>>
>> Signed-off-by: Michal Kazior <[email protected]>
>> ---
>
> Drop this single patch, please.
>
> I just noticed rx_nss doesn't seem to have correct value when associating..
Ok, I'm dropping this.
--
Kalle Valo
Calculating STA NSS just from the rateset is not
the greatest idea. The NSS could be possibly
influenced by other things that only mac80211
knows about.
Signed-off-by: Michal Kazior <[email protected]>
---
drivers/net/wireless/ath/ath10k/mac.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index b70a3b2..15eda44 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -925,7 +925,7 @@ static void ath10k_peer_assoc_h_basic(struct ath10k *ar,
else
arg->peer_listen_intval = ar->hw->conf.listen_interval;
- arg->peer_num_spatial_streams = 1;
+ arg->peer_num_spatial_streams = max_t(u32, 1, sta->rx_nss);
/*
* The assoc capabilities are available only in managed mode.
@@ -1075,7 +1075,6 @@ static void ath10k_peer_assoc_h_ht(struct ath10k *ar,
arg->peer_ht_rates.rates[n++] = i;
arg->peer_ht_rates.num_rates = n;
- arg->peer_num_spatial_streams = max((n+7) / 8, 1);
ath10k_dbg(ATH10K_DBG_MAC, "mac ht peer %pM mcs cnt %d nss %d\n",
arg->addr,
--
1.8.4.rc3
Michal Kazior <[email protected]> writes:
> This should fix possible connectivity issues upon
> changes of channel width, number of streams or
> SMPS on connected stations.
>
> An example trigger would be an action frame with
> operation mode change notification.
>
> Signed-off-by: Michal Kazior <[email protected]>
Sorry, somehow missed these on my previous review:
> + if (changed & IEEE80211_RC_BW_CHANGED) {
> + switch (sta->bandwidth) {
> + default:
> + ath10k_warn("unsupported STA BW: %d\n", sta->bandwidth);
> + case IEEE80211_STA_RX_BW_20:
> + chwidth = WMI_PEER_CHWIDTH_20MHZ;
> + break;
> + case IEEE80211_STA_RX_BW_40:
> + chwidth = WMI_PEER_CHWIDTH_40MHZ;
> + break;
> + case IEEE80211_STA_RX_BW_80:
> + chwidth = WMI_PEER_CHWIDTH_80MHZ;
> + break;
> + }
I assume that the idea here is to use WMI_PEER_CHWIDTH_20MHZ for the
default case. Actually I would prefer to avoid using default case
altogether, that way the compiler will warn if there's an enum value we
don't check. So for example you could add "case IEEE80211_STA_RX_BW_160"
which prints a warning and uses 20 MHz.
> + if (changed & IEEE80211_RC_SMPS_CHANGED) {
> + smps = WMI_PEER_SMPS_PS_NONE;
> +
> + switch (sta->smps_mode) {
> + case IEEE80211_SMPS_NUM_MODES:
> + ath10k_warn("invalid smps mode: %d\n", sta->smps_mode);
> + case IEEE80211_SMPS_AUTOMATIC:
> + case IEEE80211_SMPS_OFF:
> + smps = WMI_PEER_SMPS_PS_NONE;
> + break;
> + case IEEE80211_SMPS_STATIC:
> + smps = WMI_PEER_SMPS_STATIC;
> + break;
> + case IEEE80211_SMPS_DYNAMIC:
> + smps = WMI_PEER_SMPS_DYNAMIC;
> + break;
> + }
This is better, but I think it would be clearer to have
IEEE80211_SMPS_NUM_MODES last and not "fall through" the cases.
--
Kalle Valo
On 12/20/2013 02:21 AM, Kalle Valo wrote:
> Michal Kazior <[email protected]> writes:
>
>> This should fix possible connectivity issues upon
>> changes of channel width, number of streams or
>> SMPS on connected stations.
>>
>> An example trigger would be an action frame with
>> operation mode change notification.
>>
>> Signed-off-by: Michal Kazior <[email protected]>
>
> [...]
>
>> +static void ath10k_sta_rc_update(struct ieee80211_hw *hw,
>> + struct ieee80211_vif *vif,
>> + struct ieee80211_sta *sta,
>> + u32 changed)
>> +{
>> + struct ath10k *ar = hw->priv;
>> + struct ath10k_vif *arvif = ath10k_vif_to_arvif(vif);
>> + u32 chwidth, smps;
>> + int ret;
>> +
>
> [...]
>
>> + ret = ath10k_wmi_peer_set_param(ar, arvif->vdev_id, sta->addr,
>> + WMI_PEER_CHAN_WIDTH, chwidth);
>
> Johannes pointed out (danke!) that sta_rc_update() must be atomic, but
> all these WMI calls can sleep.
Another thing I have noticed, but have not had time to think much about,
is that when the firmware gets out of sync for whatever reason, it can
start just ignoring wmi commands and everything times out. Trying to
remove station vifs that are not currently in the firmware, perhaps due
to firmware crash, is a good way to hit this.
The system has very slow response to any number of things that
might require rtnl (or probably other locks). I think we are basically
holding rtnl or other large-scale locks through the wmi calls.
I think a good long-term solution would be to make the firmware smart enough
to return error codes instead of just ignoring (or crashing on) requests it
cannot properly handle. Maybe handling the wmi commands in a more
asynchronous manner might be a good strategy in the driver...
Thanks,
Ben
--
Ben Greear <[email protected]>
Candela Technologies Inc http://www.candelatech.com
Drivers with hardware rate control were given
sta->rx_nss set to 0. This was because rx_nss
calculation procedure was protected by hw/sw rate
control check.
Signed-off-by: Michal Kazior <[email protected]>
---
Hi Johannes,
I've re-checked the issue I've mentioned with the patch 'ath10k: use nss
provided by mac80211'.
Apparently drivers with HW rate control are only affected by this bug
and this patch is what I came up with as a quick fix. Any comments?
net/mac80211/rate.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/mac80211/rate.h b/net/mac80211/rate.h
index 5dedc56..32cdbd2 100644
--- a/net/mac80211/rate.h
+++ b/net/mac80211/rate.h
@@ -54,6 +54,8 @@ static inline void rate_control_rate_init(struct sta_info *sta)
struct ieee80211_supported_band *sband;
struct ieee80211_chanctx_conf *chanctx_conf;
+ ieee80211_sta_set_rx_nss(sta);
+
if (!ref)
return;
@@ -67,8 +69,6 @@ static inline void rate_control_rate_init(struct sta_info *sta)
sband = local->hw.wiphy->bands[chanctx_conf->def.chan->band];
- ieee80211_sta_set_rx_nss(sta);
-
ref->ops->rate_init(ref->priv, sband, &chanctx_conf->def, ista,
priv_sta);
rcu_read_unlock();
--
1.8.4.rc3
Michal Kazior <[email protected]> writes:
> This should fix possible connectivity issues upon
> changes of channel width, number of streams or
> SMPS on connected stations.
>
> An example trigger would be an action frame with
> operation mode change notification.
>
> Signed-off-by: Michal Kazior <[email protected]>
[...]
> +static void ath10k_sta_rc_update(struct ieee80211_hw *hw,
> + struct ieee80211_vif *vif,
> + struct ieee80211_sta *sta,
> + u32 changed)
> +{
> + struct ath10k *ar = hw->priv;
> + struct ath10k_vif *arvif = ath10k_vif_to_arvif(vif);
> + u32 chwidth, smps;
> + int ret;
> +
[...]
> + ret = ath10k_wmi_peer_set_param(ar, arvif->vdev_id, sta->addr,
> + WMI_PEER_CHAN_WIDTH, chwidth);
Johannes pointed out (danke!) that sta_rc_update() must be atomic, but
all these WMI calls can sleep.
--
Kalle Valo
On Mon, 2013-12-02 at 11:54 +0100, Michal Kazior wrote:
> Drivers with hardware rate control were given
> sta->rx_nss set to 0. This was because rx_nss
> calculation procedure was protected by hw/sw rate
> control check.
>
> Signed-off-by: Michal Kazior <[email protected]>
> ---
> Hi Johannes,
>
> I've re-checked the issue I've mentioned with the patch 'ath10k: use nss
> provided by mac80211'.
>
> Apparently drivers with HW rate control are only affected by this bug
> and this patch is what I came up with as a quick fix. Any comments?
Yeah this looks reasonable. I guess I'll apply it despite the [RFC]
johannes
On Mon, 2013-12-02 at 11:54 +0100, Michal Kazior wrote:
> Drivers with hardware rate control were given
> sta->rx_nss set to 0. This was because rx_nss
> calculation procedure was protected by hw/sw rate
> control check.
Applied.
johannes
This should fix possible connectivity issues upon
changes of channel width, number of streams or
SMPS on connected stations.
An example trigger would be an action frame with
operation mode change notification.
Signed-off-by: Michal Kazior <[email protected]>
---
v2:
- handle 160MHz case chwidth explicitly instead
of default case
- handle SMPS_NUM_MODES without a fall-through
drivers/net/wireless/ath/ath10k/mac.c | 91 +++++++++++++++++++++++++++++++++++
drivers/net/wireless/ath/ath10k/wmi.h | 6 +++
2 files changed, 97 insertions(+)
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index ce9ef349..ddacb1e 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -3310,6 +3310,96 @@ exit:
return ret;
}
+static void ath10k_sta_rc_update(struct ieee80211_hw *hw,
+ struct ieee80211_vif *vif,
+ struct ieee80211_sta *sta,
+ u32 changed)
+{
+ struct ath10k *ar = hw->priv;
+ struct ath10k_vif *arvif = ath10k_vif_to_arvif(vif);
+ u32 chwidth, smps;
+ int ret;
+
+ if (changed & IEEE80211_RC_BW_CHANGED) {
+ chwidth = WMI_PEER_CHWIDTH_20MHZ;
+
+ switch (sta->bandwidth) {
+ case IEEE80211_STA_RX_BW_20:
+ chwidth = WMI_PEER_CHWIDTH_20MHZ;
+ break;
+ case IEEE80211_STA_RX_BW_40:
+ chwidth = WMI_PEER_CHWIDTH_40MHZ;
+ break;
+ case IEEE80211_STA_RX_BW_80:
+ chwidth = WMI_PEER_CHWIDTH_80MHZ;
+ break;
+ case IEEE80211_STA_RX_BW_160:
+ chwidth = WMI_PEER_CHWIDTH_20MHZ;
+ ath10k_warn("unsupported STA BW: %d\n", sta->bandwidth);
+ break;
+ }
+
+ ath10k_dbg(ATH10K_DBG_MAC,
+ "mac update sta %pM bandwidth (peer chwidth %d) to %d\n",
+ sta->addr, chwidth, sta->bandwidth);
+
+ ret = ath10k_wmi_peer_set_param(ar, arvif->vdev_id, sta->addr,
+ WMI_PEER_CHAN_WIDTH, chwidth);
+ if (ret)
+ ath10k_warn("failed to update STA %pM bandwidth (peer chwidth %d) to %d: %d\n",
+ sta->addr, chwidth, sta->bandwidth, ret);
+ }
+
+ if (changed & IEEE80211_RC_NSS_CHANGED) {
+ ath10k_dbg(ATH10K_DBG_MAC, "mac update sta %pM nss to %d\n",
+ sta->addr, sta->rx_nss);
+
+ ret = ath10k_wmi_peer_set_param(ar, arvif->vdev_id, sta->addr,
+ WMI_PEER_NSS, sta->rx_nss);
+ if (ret)
+ ath10k_warn("failed to update STA %pM nss to %d: %d\n",
+ sta->addr, sta->rx_nss, ret);
+ }
+
+ if (changed & IEEE80211_RC_SMPS_CHANGED) {
+ smps = WMI_PEER_SMPS_PS_NONE;
+
+ switch (sta->smps_mode) {
+ case IEEE80211_SMPS_AUTOMATIC:
+ case IEEE80211_SMPS_OFF:
+ smps = WMI_PEER_SMPS_PS_NONE;
+ break;
+ case IEEE80211_SMPS_STATIC:
+ smps = WMI_PEER_SMPS_STATIC;
+ break;
+ case IEEE80211_SMPS_DYNAMIC:
+ smps = WMI_PEER_SMPS_DYNAMIC;
+ break;
+ case IEEE80211_SMPS_NUM_MODES:
+ smps = WMI_PEER_SMPS_PS_NONE;
+ ath10k_warn("invalid smps mode: %d\n", sta->smps_mode);
+ break;
+ }
+
+ ath10k_dbg(ATH10K_DBG_MAC,
+ "mac update sta %pM smps (peer smps %d) to %d\n",
+ sta->addr, smps, sta->smps_mode);
+
+ ret = ath10k_wmi_peer_set_param(ar, arvif->vdev_id, sta->addr,
+ WMI_PEER_SMPS_STATE, smps);
+ if (ret)
+ ath10k_warn("failed to update STA %pM smps (peer smps %d) to %d: %d\n",
+ sta->addr, smps, sta->smps_mode, ret);
+ }
+
+ if (changed & IEEE80211_RC_SUPP_RATES_CHANGED) {
+ /* FIXME: Not implemented. Not really sure if it's possible to
+ * influence HW rate control so much. */
+ ath10k_dbg(ATH10K_DBG_MAC,
+ "mac sta rc update - supp rates changed - not implemented\n");
+ }
+}
+
static const struct ieee80211_ops ath10k_ops = {
.tx = ath10k_tx,
.start = ath10k_start,
@@ -3332,6 +3422,7 @@ static const struct ieee80211_ops ath10k_ops = {
.tx_last_beacon = ath10k_tx_last_beacon,
.restart_complete = ath10k_restart_complete,
.get_survey = ath10k_get_survey,
+ .sta_rc_update = ath10k_sta_rc_update,
#ifdef CONFIG_PM
.suspend = ath10k_suspend,
.resume = ath10k_resume,
diff --git a/drivers/net/wireless/ath/ath10k/wmi.h b/drivers/net/wireless/ath/ath10k/wmi.h
index 0087d69..e8c4bb7 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.h
+++ b/drivers/net/wireless/ath/ath10k/wmi.h
@@ -3847,6 +3847,12 @@ enum wmi_peer_smps_state {
WMI_PEER_SMPS_DYNAMIC = 0x2
};
+enum wmi_peer_chwidth {
+ WMI_PEER_CHWIDTH_20MHZ = 0,
+ WMI_PEER_CHWIDTH_40MHZ = 1,
+ WMI_PEER_CHWIDTH_80MHZ = 2,
+};
+
enum wmi_peer_param {
WMI_PEER_SMPS_STATE = 0x1, /* see %wmi_peer_smps_state */
WMI_PEER_AMPDU = 0x2,
--
1.8.4.rc3