diff mbox

[PATCHv4] mac80211: enable TPC through mac80211 stack

Message ID 1420566070-25336-1-git-send-email-lorenzo.bianconi83@gmail.com (mailing list archive)
State Changes Requested
Delegated to: Johannes Berg
Headers show

Commit Message

Lorenzo Bianconi Jan. 6, 2015, 5:41 p.m. UTC
Control per packet Transmit Power Control (TPC) in lower drivers
according to TX power settings configured by the user. In particular TPC is
enabled if value passed in enum nl80211_tx_power_setting is
NL80211_TX_POWER_AUTOMATIC (no limit from userspace) or
NL80211_TX_POWER_LIMITED (allow using less than specified from userspace),
whereas TPC is disabled if nl80211_tx_power_setting is set to
NL80211_TX_POWER_FIXED (use value configured from userspace)

Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi83@gmail.com>
---
 include/net/mac80211.h     |  8 ++++++++
 net/mac80211/cfg.c         | 16 +++++++++++++---
 net/mac80211/chan.c        |  4 ++--
 net/mac80211/ieee80211_i.h |  3 ++-
 net/mac80211/iface.c       |  5 +++--
 5 files changed, 28 insertions(+), 8 deletions(-)

Comments

Johannes Berg Jan. 7, 2015, 2:12 p.m. UTC | #1
On Tue, 2015-01-06 at 18:41 +0100, Lorenzo Bianconi wrote:

> + * @txpower_type: TX power adjustment used to control per packet Transmit
> + *	Power Control (TPC) in lower driver for the current vif. In particular
> + *	TPC is enabled if value passed in %txpower_type is
> + *	NL80211_TX_POWER_AUTOMATIC (no limit from userspace) or
> + *	NL80211_TX_POWER_LIMITED (allow using less than specified from
> + *	userspace), whereas TPC is disabled if %txpower_type is set to
> + *	NL80211_TX_POWER_FIXED (use value configured from userspace)

I was going to apply this, but now I have second thoughts.

Does it really make sense to ask the driver to do "automatic"? After
all, there are other limits in mac80211 (like 11h from the AP and
regulatory) that need to be considered. So perhaps if userspace says
"automatic", the driver should still see only "limited"?

johannes

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Lorenzo Bianconi Jan. 7, 2015, 2:39 p.m. UTC | #2
> On Tue, 2015-01-06 at 18:41 +0100, Lorenzo Bianconi wrote:
>
>> + * @txpower_type: TX power adjustment used to control per packet Transmit
>> + *   Power Control (TPC) in lower driver for the current vif. In particular
>> + *   TPC is enabled if value passed in %txpower_type is
>> + *   NL80211_TX_POWER_AUTOMATIC (no limit from userspace) or
>> + *   NL80211_TX_POWER_LIMITED (allow using less than specified from
>> + *   userspace), whereas TPC is disabled if %txpower_type is set to
>> + *   NL80211_TX_POWER_FIXED (use value configured from userspace)
>
> I was going to apply this, but now I have second thoughts.
>
> Does it really make sense to ask the driver to do "automatic"? After
> all, there are other limits in mac80211 (like 11h from the AP and
> regulatory) that need to be considered. So perhaps if userspace says
> "automatic", the driver should still see only "limited"?
>

IMHO "automatic" means lower driver has to take into account just
channel/regulatory constraints (that is ath9k behavior), since the
user/mac80211 does not cap TX power, whereas "limited" means the
user/mac80211 has set a limit for emitted power, so that configuration
has to be taken into account. Maybe we have just left to set
txpower_type to NL80211_TX_POWER_LIMITED for the current vif when TX
power is limited by the AP (802.11h). What do you think? Are there any
other cases when TX power is limited by mac80211 (excluding 802.11h
and when TX power is limited by the user)?

> johannes
>

Regards,
Lorenzo
Johannes Berg Jan. 8, 2015, 7:48 a.m. UTC | #3
On Wed, 2015-01-07 at 15:39 +0100, Lorenzo Bianconi wrote:

> > Does it really make sense to ask the driver to do "automatic"? After
> > all, there are other limits in mac80211 (like 11h from the AP and
> > regulatory) that need to be considered. So perhaps if userspace says
> > "automatic", the driver should still see only "limited"?
> >
> 
> IMHO "automatic" means lower driver has to take into account just
> channel/regulatory constraints (that is ath9k behavior), since the
> user/mac80211 does not cap TX power, whereas "limited" means the
> user/mac80211 has set a limit for emitted power, so that configuration
> has to be taken into account. 

Well, mac80211 always gives you a max TX power value though - and if
it's only the maximum regulatory permitted on that channel. So there's
always a valid upper bound - unlike the userspace API where "automatic"
means that you have no idea of an upper bound. Inside the kernel you
always have an upper bound, so "automatic" doesn't make that much sense?

johannes

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Lorenzo Bianconi Jan. 8, 2015, 11:07 a.m. UTC | #4
> On Wed, 2015-01-07 at 15:39 +0100, Lorenzo Bianconi wrote:
>
>> > Does it really make sense to ask the driver to do "automatic"? After
>> > all, there are other limits in mac80211 (like 11h from the AP and
>> > regulatory) that need to be considered. So perhaps if userspace says
>> > "automatic", the driver should still see only "limited"?
>> >
>>
>> IMHO "automatic" means lower driver has to take into account just
>> channel/regulatory constraints (that is ath9k behavior), since the
>> user/mac80211 does not cap TX power, whereas "limited" means the
>> user/mac80211 has set a limit for emitted power, so that configuration
>> has to be taken into account.
>
> Well, mac80211 always gives you a max TX power value though - and if
> it's only the maximum regulatory permitted on that channel. So there's
> always a valid upper bound - unlike the userspace API where "automatic"
> means that you have no idea of an upper bound. Inside the kernel you
> always have an upper bound, so "automatic" doesn't make that much sense?
>

Sounds good, now I have clearer vision :)
I will overwrite bss_conf nl80211_tx_power_setting value in
ieee80211_set_tx_power() when txpower type is set to
NL80211_TX_POWER_AUTOMATIC from userspace. Moreover should we set
nl80211_tx_power_setting to NL80211_TX_POWER_LIMITED in
ieee80211_handle_pwr_constr() when the station receives a power
constraint in beacon frames? What do you think?

> johannes
>

Regards,
Lorenzo
Johannes Berg Jan. 14, 2015, 8:29 a.m. UTC | #5
Sorry ...

On Thu, 2015-01-08 at 12:07 +0100, Lorenzo Bianconi wrote:

> Sounds good, now I have clearer vision :)
> I will overwrite bss_conf nl80211_tx_power_setting value in
> ieee80211_set_tx_power() when txpower type is set to
> NL80211_TX_POWER_AUTOMATIC from userspace. Moreover should we set
> nl80211_tx_power_setting to NL80211_TX_POWER_LIMITED in
> ieee80211_handle_pwr_constr() when the station receives a power
> constraint in beacon frames? What do you think?

Isn't it basically always only valid to have LIMITED and FIXED? I don't
see much value in fixed really though, but if the user requested
that ...

johannes

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 4913c00..e47fdbc 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -376,6 +376,13 @@  enum ieee80211_rssi_event {
  * @ssid_len: Length of SSID given in @ssid.
  * @hidden_ssid: The SSID of the current vif is hidden. Only valid in AP-mode.
  * @txpower: TX power in dBm
+ * @txpower_type: TX power adjustment used to control per packet Transmit
+ *	Power Control (TPC) in lower driver for the current vif. In particular
+ *	TPC is enabled if value passed in %txpower_type is
+ *	NL80211_TX_POWER_AUTOMATIC (no limit from userspace) or
+ *	NL80211_TX_POWER_LIMITED (allow using less than specified from
+ *	userspace), whereas TPC is disabled if %txpower_type is set to
+ *	NL80211_TX_POWER_FIXED (use value configured from userspace)
  * @p2p_noa_attr: P2P NoA attribute for P2P powersave
  */
 struct ieee80211_bss_conf {
@@ -411,6 +418,7 @@  struct ieee80211_bss_conf {
 	size_t ssid_len;
 	bool hidden_ssid;
 	int txpower;
+	enum nl80211_tx_power_setting txpower_type;
 	struct ieee80211_p2p_noa_attr p2p_noa_attr;
 };
 
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 1696658..9c7e1ee 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -2103,6 +2103,7 @@  static int ieee80211_set_tx_power(struct wiphy *wiphy,
 {
 	struct ieee80211_local *local = wiphy_priv(wiphy);
 	struct ieee80211_sub_if_data *sdata;
+	bool update_txp_type = false;
 
 	if (wdev) {
 		sdata = IEEE80211_WDEV_TO_SUB_IF(wdev);
@@ -2119,7 +2120,12 @@  static int ieee80211_set_tx_power(struct wiphy *wiphy,
 			break;
 		}
 
-		ieee80211_recalc_txpower(sdata);
+		if (type != sdata->vif.bss_conf.txpower_type) {
+			update_txp_type = true;
+			sdata->vif.bss_conf.txpower_type = type;
+		}
+
+		ieee80211_recalc_txpower(sdata, update_txp_type);
 
 		return 0;
 	}
@@ -2137,10 +2143,14 @@  static int ieee80211_set_tx_power(struct wiphy *wiphy,
 	}
 
 	mutex_lock(&local->iflist_mtx);
-	list_for_each_entry(sdata, &local->interfaces, list)
+	list_for_each_entry(sdata, &local->interfaces, list) {
 		sdata->user_power_level = local->user_power_level;
+		if (type != sdata->vif.bss_conf.txpower_type)
+			update_txp_type = true;
+		sdata->vif.bss_conf.txpower_type = type;
+	}
 	list_for_each_entry(sdata, &local->interfaces, list)
-		ieee80211_recalc_txpower(sdata);
+		ieee80211_recalc_txpower(sdata, update_txp_type);
 	mutex_unlock(&local->iflist_mtx);
 
 	return 0;
diff --git a/net/mac80211/chan.c b/net/mac80211/chan.c
index 5d6dae9..a881370 100644
--- a/net/mac80211/chan.c
+++ b/net/mac80211/chan.c
@@ -628,7 +628,7 @@  out:
 	}
 
 	if (new_ctx && ieee80211_chanctx_num_assigned(local, new_ctx) > 0) {
-		ieee80211_recalc_txpower(sdata);
+		ieee80211_recalc_txpower(sdata, false);
 		ieee80211_recalc_chanctx_min_def(local, new_ctx);
 	}
 
@@ -1356,7 +1356,7 @@  static int ieee80211_vif_use_reserved_switch(struct ieee80211_local *local)
 				ieee80211_bss_info_change_notify(sdata,
 								 changed);
 
-			ieee80211_recalc_txpower(sdata);
+			ieee80211_recalc_txpower(sdata, false);
 		}
 
 		ieee80211_recalc_chanctx_chantype(local, ctx);
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 4f45cab..486dbc5d 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -1623,7 +1623,8 @@  int ieee80211_add_virtual_monitor(struct ieee80211_local *local);
 void ieee80211_del_virtual_monitor(struct ieee80211_local *local);
 
 bool __ieee80211_recalc_txpower(struct ieee80211_sub_if_data *sdata);
-void ieee80211_recalc_txpower(struct ieee80211_sub_if_data *sdata);
+void ieee80211_recalc_txpower(struct ieee80211_sub_if_data *sdata,
+			      bool update_bss);
 
 static inline bool ieee80211_sdata_running(struct ieee80211_sub_if_data *sdata)
 {
diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index 4173553..bc885e2 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -73,9 +73,10 @@  bool __ieee80211_recalc_txpower(struct ieee80211_sub_if_data *sdata)
 	return false;
 }
 
-void ieee80211_recalc_txpower(struct ieee80211_sub_if_data *sdata)
+void ieee80211_recalc_txpower(struct ieee80211_sub_if_data *sdata,
+			      bool update_bss)
 {
-	if (__ieee80211_recalc_txpower(sdata))
+	if (__ieee80211_recalc_txpower(sdata) || update_bss)
 		ieee80211_bss_info_change_notify(sdata, BSS_CHANGED_TXPOWER);
 }