All of lore.kernel.org
 help / color / mirror / Atom feed
From: Bob Copeland <me@bobcopeland.com>
To: Sven Eckelmann <seckelmann@datto.com>
Cc: linux-wireless@vger.kernel.org, ath11k@lists.infradead.org
Subject: Re: [RFC PATCH v2 3/3] ath11k: register HE mesh capabilities
Date: Wed, 12 Jun 2019 12:34:57 -0400	[thread overview]
Message-ID: <20190612163457.GB4090@localhost> (raw)
In-Reply-To: <1919330.hVZVHELXip@bentobox>

On Tue, Jun 11, 2019 at 09:52:20PM +0200, Sven Eckelmann wrote:
> On Tuesday, 11 June 2019 20:02:47 CEST Sven Eckelmann wrote:
> [...]
> > ---
> > This doesn't work currently as expected. No HE rates are used between
> > the two HE mesh peers:
> [...]
> 
> There seems to be also an ordering problem. ath11k_peer_assoc_h_he is only 
> called before ieee80211_he_cap_ie_to_sta_he_cap is called. So ath11k_bss_assoc 
> will not have the information whether the remote has HE support or not.
> 
> Looks like I have adjust mesh_sta_info_init to get this somehow to 
> ath11k_peer_assoc_h_he. Maybe through ath11k_sta_rc_update but this is not 
> called by mesh_sta_info_init at the moment. Just because 
> rate_control_rate_init is called and not rate_control_rate_update.
> 
> The easiest method seems to adjust the check at the end of mesh_sta_info_init 
> to 
> 
> 	if (!test_sta_flag(sta, WLAN_STA_RATE_CONTROL) &&
> 	    !ieee80211_hw_check(&local->hw, HAS_RATE_CONTROL)) {
> 		rate_control_rate_init(sta);
> 	} else {
> 		rate_control_rate_update(local, sband, sta, changed);
> 	}

Maybe we should just do this?

diff --git a/net/mac80211/rate.c b/net/mac80211/rate.c
index 76f303fda3ed..6f8bde840bb9 100644
--- a/net/mac80211/rate.c
+++ b/net/mac80211/rate.c
@@ -42,7 +42,7 @@ void rate_control_rate_init(struct sta_info *sta)
 	ieee80211_sta_set_rx_nss(sta);
 
 	if (!ref)
-		return;
+		goto out;
 
 	rcu_read_lock();
 
@@ -59,6 +59,7 @@ void rate_control_rate_init(struct sta_info *sta)
 			    priv_sta);
 	spin_unlock_bh(&sta->rate_ctrl_lock);
 	rcu_read_unlock();
+out:
 	set_sta_flag(sta, WLAN_STA_RATE_CONTROL);
 }


That was my intent, anyway -- that NSS always got set before
rate_control_rate_update() even if using HW rate control.

> 	if (!test_sta_flag(sta, WLAN_STA_RATE_CONTROL))
> 		rate_control_rate_init(sta);
> 
>    /* inform drivers about changes */
> 	rate_control_rate_update(local, sband, sta, changed);
> 
> Both will at least cause a call to ath11k_peer_assoc_prepare + 
> ath11k_wmi_send_peer_assoc_cmd but unfortunately the ath11k firmware hangs 
> afterwards.

I think this would be OK.

-- 
Bob Copeland %% https://bobcopeland.com/

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

WARNING: multiple messages have this Message-ID (diff)
From: Bob Copeland <me@bobcopeland.com>
To: Sven Eckelmann <seckelmann@datto.com>
Cc: ath11k@lists.infradead.org, linux-wireless@vger.kernel.org
Subject: Re: [RFC PATCH v2 3/3] ath11k: register HE mesh capabilities
Date: Wed, 12 Jun 2019 12:34:57 -0400	[thread overview]
Message-ID: <20190612163457.GB4090@localhost> (raw)
In-Reply-To: <1919330.hVZVHELXip@bentobox>

On Tue, Jun 11, 2019 at 09:52:20PM +0200, Sven Eckelmann wrote:
> On Tuesday, 11 June 2019 20:02:47 CEST Sven Eckelmann wrote:
> [...]
> > ---
> > This doesn't work currently as expected. No HE rates are used between
> > the two HE mesh peers:
> [...]
> 
> There seems to be also an ordering problem. ath11k_peer_assoc_h_he is only 
> called before ieee80211_he_cap_ie_to_sta_he_cap is called. So ath11k_bss_assoc 
> will not have the information whether the remote has HE support or not.
> 
> Looks like I have adjust mesh_sta_info_init to get this somehow to 
> ath11k_peer_assoc_h_he. Maybe through ath11k_sta_rc_update but this is not 
> called by mesh_sta_info_init at the moment. Just because 
> rate_control_rate_init is called and not rate_control_rate_update.
> 
> The easiest method seems to adjust the check at the end of mesh_sta_info_init 
> to 
> 
> 	if (!test_sta_flag(sta, WLAN_STA_RATE_CONTROL) &&
> 	    !ieee80211_hw_check(&local->hw, HAS_RATE_CONTROL)) {
> 		rate_control_rate_init(sta);
> 	} else {
> 		rate_control_rate_update(local, sband, sta, changed);
> 	}

Maybe we should just do this?

diff --git a/net/mac80211/rate.c b/net/mac80211/rate.c
index 76f303fda3ed..6f8bde840bb9 100644
--- a/net/mac80211/rate.c
+++ b/net/mac80211/rate.c
@@ -42,7 +42,7 @@ void rate_control_rate_init(struct sta_info *sta)
 	ieee80211_sta_set_rx_nss(sta);
 
 	if (!ref)
-		return;
+		goto out;
 
 	rcu_read_lock();
 
@@ -59,6 +59,7 @@ void rate_control_rate_init(struct sta_info *sta)
 			    priv_sta);
 	spin_unlock_bh(&sta->rate_ctrl_lock);
 	rcu_read_unlock();
+out:
 	set_sta_flag(sta, WLAN_STA_RATE_CONTROL);
 }


That was my intent, anyway -- that NSS always got set before
rate_control_rate_update() even if using HW rate control.

> 	if (!test_sta_flag(sta, WLAN_STA_RATE_CONTROL))
> 		rate_control_rate_init(sta);
> 
>    /* inform drivers about changes */
> 	rate_control_rate_update(local, sband, sta, changed);
> 
> Both will at least cause a call to ath11k_peer_assoc_prepare + 
> ath11k_wmi_send_peer_assoc_cmd but unfortunately the ath11k firmware hangs 
> afterwards.

I think this would be OK.

-- 
Bob Copeland %% https://bobcopeland.com/

  parent reply	other threads:[~2019-06-12 16:35 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-11 18:02 [RFC PATCH v2 0/3] mac80211/ath11k: HE mesh support Sven Eckelmann
2019-06-11 18:02 ` Sven Eckelmann
2019-06-11 18:02 ` [RFC PATCH v2 1/3] mac80211_hwsim: Register support for HE meshpoint Sven Eckelmann
2019-06-11 18:02   ` Sven Eckelmann
2019-06-11 18:02 ` [RFC PATCH v2 2/3] mac80211: implement HE support for mesh Sven Eckelmann
2019-06-11 18:02   ` Sven Eckelmann
2019-06-11 18:02 ` [RFC PATCH v2 3/3] ath11k: register HE mesh capabilities Sven Eckelmann
2019-06-11 18:02   ` Sven Eckelmann
2019-06-11 19:52   ` Sven Eckelmann
2019-06-11 19:52     ` Sven Eckelmann
2019-06-12  8:42     ` Sven Eckelmann
2019-06-12  8:42       ` Sven Eckelmann
2019-06-12 16:34     ` Bob Copeland [this message]
2019-06-12 16:34       ` Bob Copeland

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190612163457.GB4090@localhost \
    --to=me@bobcopeland.com \
    --cc=ath11k@lists.infradead.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=seckelmann@datto.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.