[RFC] mac80211: Fix setting txpower to zero.

[RFC] mac80211: Fix setting txpower to zero.

[greearb]

From: Ben Greear <greearb@candelatech.com>

With multiple VIFS ath10k, and probably others, tries to find the
minimum txpower for all vifs and uses that when setting txpower in
the firmware.

If a second vif is added and starts to scan, it's txpower is not initialized yet
and it set to zero.

ath10k had a patch to ignore zero values, but then it is impossible to actually set
txpower to zero.

So, instead initialize the txpower to -1 in mac80211, and let drivers know that
means the power has not been set and so should be ignored.

This should fix regression in:

commit 88407beb1b1462f706a1950a355fd086e1c450b6
Author: Ryan Hsu <ryanhsu@qca.qualcomm.com>
Date:   Tue Dec 13 14:55:19 2016 -0800

    ath10k: fix incorrect txpower set by P2P_DEVICE interface

Tested on ath10k 9984 with ath10k-ct firmware.

Signed-off-by: Ben Greear <greearb@candelatech.com>
---
 drivers/net/wireless/ath/ath10k/mac.c | 2 +-
 drivers/net/wireless/ath/ath9k/main.c | 3 +++
 drivers/net/wireless/ath/ath9k/xmit.c | 7 +++++--
 include/net/mac80211.h                | 2 +-
 net/mac80211/iface.c                  | 1 +
 net/mac80211/main.c                   | 2 ++
 6 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 289d03da14b2..c846f232e930 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -5906,7 +5906,7 @@ static int ath10k_mac_txpower_recalc(struct ath10k *ar)
 	lockdep_assert_held(&ar->conf_mutex);
 
 	list_for_each_entry(arvif, &ar->arvifs, list) {
-		if (arvif->txpower <= 0)
+		if (arvif->txpower < 0) /* txpower not initialized yet? */
 			continue;
 
 		if (txpower == -1)
diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index 14f253199909..2edf70cf7e7e 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -1196,6 +1196,9 @@ static void ath9k_tpc_vif_iter(void *data, u8 *mac, struct ieee80211_vif *vif)
 {
 	int *power = data;
 
+	if (vif->bss_conf.txpower < 0)
+		return;
+
 	if (*power < vif->bss_conf.txpower)
 		*power = vif->bss_conf.txpower;
 }
diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
index 751d0d0550b5..82c592ca2cd2 100644
--- a/drivers/net/wireless/ath/ath9k/xmit.c
+++ b/drivers/net/wireless/ath/ath9k/xmit.c
@@ -2114,10 +2114,13 @@ static void setup_frame_info(struct ieee80211_hw *hw,
 
 	if (tx_info->control.vif) {
 		struct ieee80211_vif *vif = tx_info->control.vif;
-
+		if (vif->bss_conf.txpower < 0)
+			goto nonvifpower;
 		txpower = 2 * vif->bss_conf.txpower;
 	} else {
-		struct ath_softc *sc = hw->priv;
+		struct ath_softc *sc;
+	nonvifpower:
+		sc = hw->priv;
 
 		txpower = sc->cur_chan->cur_txpower;
 	}
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 2b70b9268f76..db66520c5389 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -569,7 +569,7 @@ struct ieee80211_ftm_responder_params {
  * @ssid: The SSID of the current vif. Valid in AP and IBSS mode.
  * @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: TX power in dBm.  -1 means not configured.
  * @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
diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index b0c2df6e22c5..49fcf9d80f85 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -1459,6 +1459,7 @@ static void ieee80211_setup_sdata(struct ieee80211_sub_if_data *sdata,
 	sdata->control_port_no_encrypt = false;
 	sdata->encrypt_headroom = IEEE80211_ENCRYPT_HEADROOM;
 	sdata->vif.bss_conf.idle = true;
+	sdata->vif.bss_conf.txpower = -1; /* unset */
 
 	sdata->noack_map = 0;
 
diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index a148509a88bc..2f53188851ee 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -145,6 +145,8 @@ static u32 ieee80211_hw_conf_chan(struct ieee80211_local *local)
 			continue;
 		if (sdata->vif.type == NL80211_IFTYPE_AP_VLAN)
 			continue;
+		if (sdata->vif.bss_conf.txpower < 0)
+			continue;
 		power = min(power, sdata->vif.bss_conf.txpower);
 	}
 	rcu_read_unlock();
-- 
2.20.1

Re: [RFC] mac80211: Fix setting txpower to zero.

[Ben Greear]

On 12/13/19 3:03 PM, greearb@candelatech.com wrote:
> From: Ben Greear <greearb@candelatech.com>
> 
> With multiple VIFS ath10k, and probably others, tries to find the
> minimum txpower for all vifs and uses that when setting txpower in
> the firmware.

Johannes, sorry to be impatient, but I want to get some new ath10k-ct
changes into OpenWRT (to fix per-chain RSSI reporting), and if only my
driver change goes in, then ath10k-ct will have the old problem that Ryan Hsu
tried to fix.

Are you OK with initializing the txpower to -1 to mean 'unset' in mac80211?  Or, do I need
to come up with some other way to indicate to the driver that the txpower should
be ignored?

Thanks,
Ben

> 
> If a second vif is added and starts to scan, it's txpower is not initialized yet
> and it set to zero.
> 
> ath10k had a patch to ignore zero values, but then it is impossible to actually set
> txpower to zero.
> 
> So, instead initialize the txpower to -1 in mac80211, and let drivers know that
> means the power has not been set and so should be ignored.
> 
> This should fix regression in:
> 
> commit 88407beb1b1462f706a1950a355fd086e1c450b6
> Author: Ryan Hsu <ryanhsu@qca.qualcomm.com>
> Date:   Tue Dec 13 14:55:19 2016 -0800
> 
>      ath10k: fix incorrect txpower set by P2P_DEVICE interface
> 
> Tested on ath10k 9984 with ath10k-ct firmware.
> 
> Signed-off-by: Ben Greear <greearb@candelatech.com>
> ---
>   drivers/net/wireless/ath/ath10k/mac.c | 2 +-
>   drivers/net/wireless/ath/ath9k/main.c | 3 +++
>   drivers/net/wireless/ath/ath9k/xmit.c | 7 +++++--
>   include/net/mac80211.h                | 2 +-
>   net/mac80211/iface.c                  | 1 +
>   net/mac80211/main.c                   | 2 ++
>   6 files changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
> index 289d03da14b2..c846f232e930 100644
> --- a/drivers/net/wireless/ath/ath10k/mac.c
> +++ b/drivers/net/wireless/ath/ath10k/mac.c
> @@ -5906,7 +5906,7 @@ static int ath10k_mac_txpower_recalc(struct ath10k *ar)
>   	lockdep_assert_held(&ar->conf_mutex);
>   
>   	list_for_each_entry(arvif, &ar->arvifs, list) {
> -		if (arvif->txpower <= 0)
> +		if (arvif->txpower < 0) /* txpower not initialized yet? */
>   			continue;
>   
>   		if (txpower == -1)
> diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
> index 14f253199909..2edf70cf7e7e 100644
> --- a/drivers/net/wireless/ath/ath9k/main.c
> +++ b/drivers/net/wireless/ath/ath9k/main.c
> @@ -1196,6 +1196,9 @@ static void ath9k_tpc_vif_iter(void *data, u8 *mac, struct ieee80211_vif *vif)
>   {
>   	int *power = data;
>   
> +	if (vif->bss_conf.txpower < 0)
> +		return;
> +
>   	if (*power < vif->bss_conf.txpower)
>   		*power = vif->bss_conf.txpower;
>   }
> diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
> index 751d0d0550b5..82c592ca2cd2 100644
> --- a/drivers/net/wireless/ath/ath9k/xmit.c
> +++ b/drivers/net/wireless/ath/ath9k/xmit.c
> @@ -2114,10 +2114,13 @@ static void setup_frame_info(struct ieee80211_hw *hw,
>   
>   	if (tx_info->control.vif) {
>   		struct ieee80211_vif *vif = tx_info->control.vif;
> -
> +		if (vif->bss_conf.txpower < 0)
> +			goto nonvifpower;
>   		txpower = 2 * vif->bss_conf.txpower;
>   	} else {
> -		struct ath_softc *sc = hw->priv;
> +		struct ath_softc *sc;
> +	nonvifpower:
> +		sc = hw->priv;
>   
>   		txpower = sc->cur_chan->cur_txpower;
>   	}
> diff --git a/include/net/mac80211.h b/include/net/mac80211.h
> index 2b70b9268f76..db66520c5389 100644
> --- a/include/net/mac80211.h
> +++ b/include/net/mac80211.h
> @@ -569,7 +569,7 @@ struct ieee80211_ftm_responder_params {
>    * @ssid: The SSID of the current vif. Valid in AP and IBSS mode.
>    * @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: TX power in dBm.  -1 means not configured.
>    * @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
> diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
> index b0c2df6e22c5..49fcf9d80f85 100644
> --- a/net/mac80211/iface.c
> +++ b/net/mac80211/iface.c
> @@ -1459,6 +1459,7 @@ static void ieee80211_setup_sdata(struct ieee80211_sub_if_data *sdata,
>   	sdata->control_port_no_encrypt = false;
>   	sdata->encrypt_headroom = IEEE80211_ENCRYPT_HEADROOM;
>   	sdata->vif.bss_conf.idle = true;
> +	sdata->vif.bss_conf.txpower = -1; /* unset */
>   
>   	sdata->noack_map = 0;
>   
> diff --git a/net/mac80211/main.c b/net/mac80211/main.c
> index a148509a88bc..2f53188851ee 100644
> --- a/net/mac80211/main.c
> +++ b/net/mac80211/main.c
> @@ -145,6 +145,8 @@ static u32 ieee80211_hw_conf_chan(struct ieee80211_local *local)
>   			continue;
>   		if (sdata->vif.type == NL80211_IFTYPE_AP_VLAN)
>   			continue;
> +		if (sdata->vif.bss_conf.txpower < 0)
> +			continue;
>   		power = min(power, sdata->vif.bss_conf.txpower);
>   	}
>   	rcu_read_unlock();
> 


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: [RFC] mac80211: Fix setting txpower to zero.

[Justin Capella]

https://patchwork.kernel.org/patch/9465291/ looks like this older
related patch was to work around p2p control vif being set to zero and
never reconfigured, I'll try and test this patchset with various vif
types. One thing I thought of, it might be cleaner to use int max,
since code is already looking for a minimum, and ultimately would be
limited by 2g/5g power limit,  perhaps initialize the txpower with
those values? Then you avoid a magic value?

What does the hw do with a txpower of zero anyway?

On Mon, Dec 16, 2019 at 2:56 PM Ben Greear <greearb@candelatech.com> wrote:
>
> On 12/13/19 3:03 PM, greearb@candelatech.com wrote:
> > From: Ben Greear <greearb@candelatech.com>
> >
> > With multiple VIFS ath10k, and probably others, tries to find the
> > minimum txpower for all vifs and uses that when setting txpower in
> > the firmware.
>
> Johannes, sorry to be impatient, but I want to get some new ath10k-ct
> changes into OpenWRT (to fix per-chain RSSI reporting), and if only my
> driver change goes in, then ath10k-ct will have the old problem that Ryan Hsu
> tried to fix.
>
> Are you OK with initializing the txpower to -1 to mean 'unset' in mac80211?  Or, do I need
> to come up with some other way to indicate to the driver that the txpower should
> be ignored?
>
> Thanks,
> Ben
>
> >
> > If a second vif is added and starts to scan, it's txpower is not initialized yet
> > and it set to zero.
> >
> > ath10k had a patch to ignore zero values, but then it is impossible to actually set
> > txpower to zero.
> >
> > So, instead initialize the txpower to -1 in mac80211, and let drivers know that
> > means the power has not been set and so should be ignored.
> >
> > This should fix regression in:
> >
> > commit 88407beb1b1462f706a1950a355fd086e1c450b6
> > Author: Ryan Hsu <ryanhsu@qca.qualcomm.com>
> > Date:   Tue Dec 13 14:55:19 2016 -0800
> >
> >      ath10k: fix incorrect txpower set by P2P_DEVICE interface
> >
> > Tested on ath10k 9984 with ath10k-ct firmware.
> >
> > Signed-off-by: Ben Greear <greearb@candelatech.com>
> > ---
> >   drivers/net/wireless/ath/ath10k/mac.c | 2 +-
> >   drivers/net/wireless/ath/ath9k/main.c | 3 +++
> >   drivers/net/wireless/ath/ath9k/xmit.c | 7 +++++--
> >   include/net/mac80211.h                | 2 +-
> >   net/mac80211/iface.c                  | 1 +
> >   net/mac80211/main.c                   | 2 ++
> >   6 files changed, 13 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
> > index 289d03da14b2..c846f232e930 100644
> > --- a/drivers/net/wireless/ath/ath10k/mac.c
> > +++ b/drivers/net/wireless/ath/ath10k/mac.c
> > @@ -5906,7 +5906,7 @@ static int ath10k_mac_txpower_recalc(struct ath10k *ar)
> >       lockdep_assert_held(&ar->conf_mutex);
> >
> >       list_for_each_entry(arvif, &ar->arvifs, list) {
> > -             if (arvif->txpower <= 0)
> > +             if (arvif->txpower < 0) /* txpower not initialized yet? */
> >                       continue;
> >
> >               if (txpower == -1)
> > diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
> > index 14f253199909..2edf70cf7e7e 100644
> > --- a/drivers/net/wireless/ath/ath9k/main.c
> > +++ b/drivers/net/wireless/ath/ath9k/main.c
> > @@ -1196,6 +1196,9 @@ static void ath9k_tpc_vif_iter(void *data, u8 *mac, struct ieee80211_vif *vif)
> >   {
> >       int *power = data;
> >
> > +     if (vif->bss_conf.txpower < 0)
> > +             return;
> > +
> >       if (*power < vif->bss_conf.txpower)
> >               *power = vif->bss_conf.txpower;
> >   }
> > diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
> > index 751d0d0550b5..82c592ca2cd2 100644
> > --- a/drivers/net/wireless/ath/ath9k/xmit.c
> > +++ b/drivers/net/wireless/ath/ath9k/xmit.c
> > @@ -2114,10 +2114,13 @@ static void setup_frame_info(struct ieee80211_hw *hw,
> >
> >       if (tx_info->control.vif) {
> >               struct ieee80211_vif *vif = tx_info->control.vif;
> > -
> > +             if (vif->bss_conf.txpower < 0)
> > +                     goto nonvifpower;
> >               txpower = 2 * vif->bss_conf.txpower;
> >       } else {
> > -             struct ath_softc *sc = hw->priv;
> > +             struct ath_softc *sc;
> > +     nonvifpower:
> > +             sc = hw->priv;
> >
> >               txpower = sc->cur_chan->cur_txpower;
> >       }
> > diff --git a/include/net/mac80211.h b/include/net/mac80211.h
> > index 2b70b9268f76..db66520c5389 100644
> > --- a/include/net/mac80211.h
> > +++ b/include/net/mac80211.h
> > @@ -569,7 +569,7 @@ struct ieee80211_ftm_responder_params {
> >    * @ssid: The SSID of the current vif. Valid in AP and IBSS mode.
> >    * @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: TX power in dBm.  -1 means not configured.
> >    * @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
> > diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
> > index b0c2df6e22c5..49fcf9d80f85 100644
> > --- a/net/mac80211/iface.c
> > +++ b/net/mac80211/iface.c
> > @@ -1459,6 +1459,7 @@ static void ieee80211_setup_sdata(struct ieee80211_sub_if_data *sdata,
> >       sdata->control_port_no_encrypt = false;
> >       sdata->encrypt_headroom = IEEE80211_ENCRYPT_HEADROOM;
> >       sdata->vif.bss_conf.idle = true;
> > +     sdata->vif.bss_conf.txpower = -1; /* unset */
> >
> >       sdata->noack_map = 0;
> >
> > diff --git a/net/mac80211/main.c b/net/mac80211/main.c
> > index a148509a88bc..2f53188851ee 100644
> > --- a/net/mac80211/main.c
> > +++ b/net/mac80211/main.c
> > @@ -145,6 +145,8 @@ static u32 ieee80211_hw_conf_chan(struct ieee80211_local *local)
> >                       continue;
> >               if (sdata->vif.type == NL80211_IFTYPE_AP_VLAN)
> >                       continue;
> > +             if (sdata->vif.bss_conf.txpower < 0)
> > +                     continue;
> >               power = min(power, sdata->vif.bss_conf.txpower);
> >       }
> >       rcu_read_unlock();
> >
>
>
> --
> Ben Greear <greearb@candelatech.com>
> Candela Technologies Inc  http://www.candelatech.com
>

Re: [RFC] mac80211: Fix setting txpower to zero.

[Ben Greear]

On 12/16/2019 08:44 PM, Justin Capella wrote:
> https://patchwork.kernel.org/patch/9465291/ looks like this older
> related patch was to work around p2p control vif being set to zero and
> never reconfigured, I'll try and test this patchset with various vif
> types. One thing I thought of, it might be cleaner to use int max,
> since code is already looking for a minimum, and ultimately would be
> limited by 2g/5g power limit,  perhaps initialize the txpower with
> those values? Then you avoid a magic value?

I think ath9k might check for a maximum, in which case the INT_MAX becomes
a magic value, but even so, your idea has merit.

>
> What does the hw do with a txpower of zero anyway?

It transmits with one dbm less of power than txpower 1.  0 dbm does not mean no energy at all.

Thanks,
Ben

>
> On Mon, Dec 16, 2019 at 2:56 PM Ben Greear <greearb@candelatech.com> wrote:
>>
>> On 12/13/19 3:03 PM, greearb@candelatech.com wrote:
>>> From: Ben Greear <greearb@candelatech.com>
>>>
>>> With multiple VIFS ath10k, and probably others, tries to find the
>>> minimum txpower for all vifs and uses that when setting txpower in
>>> the firmware.
>>
>> Johannes, sorry to be impatient, but I want to get some new ath10k-ct
>> changes into OpenWRT (to fix per-chain RSSI reporting), and if only my
>> driver change goes in, then ath10k-ct will have the old problem that Ryan Hsu
>> tried to fix.
>>
>> Are you OK with initializing the txpower to -1 to mean 'unset' in mac80211?  Or, do I need
>> to come up with some other way to indicate to the driver that the txpower should
>> be ignored?
>>
>> Thanks,
>> Ben
>>
>>>
>>> If a second vif is added and starts to scan, it's txpower is not initialized yet
>>> and it set to zero.
>>>
>>> ath10k had a patch to ignore zero values, but then it is impossible to actually set
>>> txpower to zero.
>>>
>>> So, instead initialize the txpower to -1 in mac80211, and let drivers know that
>>> means the power has not been set and so should be ignored.
>>>
>>> This should fix regression in:
>>>
>>> commit 88407beb1b1462f706a1950a355fd086e1c450b6
>>> Author: Ryan Hsu <ryanhsu@qca.qualcomm.com>
>>> Date:   Tue Dec 13 14:55:19 2016 -0800
>>>
>>>      ath10k: fix incorrect txpower set by P2P_DEVICE interface
>>>
>>> Tested on ath10k 9984 with ath10k-ct firmware.
>>>
>>> Signed-off-by: Ben Greear <greearb@candelatech.com>
>>> ---
>>>   drivers/net/wireless/ath/ath10k/mac.c | 2 +-
>>>   drivers/net/wireless/ath/ath9k/main.c | 3 +++
>>>   drivers/net/wireless/ath/ath9k/xmit.c | 7 +++++--
>>>   include/net/mac80211.h                | 2 +-
>>>   net/mac80211/iface.c                  | 1 +
>>>   net/mac80211/main.c                   | 2 ++
>>>   6 files changed, 13 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
>>> index 289d03da14b2..c846f232e930 100644
>>> --- a/drivers/net/wireless/ath/ath10k/mac.c
>>> +++ b/drivers/net/wireless/ath/ath10k/mac.c
>>> @@ -5906,7 +5906,7 @@ static int ath10k_mac_txpower_recalc(struct ath10k *ar)
>>>       lockdep_assert_held(&ar->conf_mutex);
>>>
>>>       list_for_each_entry(arvif, &ar->arvifs, list) {
>>> -             if (arvif->txpower <= 0)
>>> +             if (arvif->txpower < 0) /* txpower not initialized yet? */
>>>                       continue;
>>>
>>>               if (txpower == -1)
>>> diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
>>> index 14f253199909..2edf70cf7e7e 100644
>>> --- a/drivers/net/wireless/ath/ath9k/main.c
>>> +++ b/drivers/net/wireless/ath/ath9k/main.c
>>> @@ -1196,6 +1196,9 @@ static void ath9k_tpc_vif_iter(void *data, u8 *mac, struct ieee80211_vif *vif)
>>>   {
>>>       int *power = data;
>>>
>>> +     if (vif->bss_conf.txpower < 0)
>>> +             return;
>>> +
>>>       if (*power < vif->bss_conf.txpower)
>>>               *power = vif->bss_conf.txpower;
>>>   }
>>> diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
>>> index 751d0d0550b5..82c592ca2cd2 100644
>>> --- a/drivers/net/wireless/ath/ath9k/xmit.c
>>> +++ b/drivers/net/wireless/ath/ath9k/xmit.c
>>> @@ -2114,10 +2114,13 @@ static void setup_frame_info(struct ieee80211_hw *hw,
>>>
>>>       if (tx_info->control.vif) {
>>>               struct ieee80211_vif *vif = tx_info->control.vif;
>>> -
>>> +             if (vif->bss_conf.txpower < 0)
>>> +                     goto nonvifpower;
>>>               txpower = 2 * vif->bss_conf.txpower;
>>>       } else {
>>> -             struct ath_softc *sc = hw->priv;
>>> +             struct ath_softc *sc;
>>> +     nonvifpower:
>>> +             sc = hw->priv;
>>>
>>>               txpower = sc->cur_chan->cur_txpower;
>>>       }
>>> diff --git a/include/net/mac80211.h b/include/net/mac80211.h
>>> index 2b70b9268f76..db66520c5389 100644
>>> --- a/include/net/mac80211.h
>>> +++ b/include/net/mac80211.h
>>> @@ -569,7 +569,7 @@ struct ieee80211_ftm_responder_params {
>>>    * @ssid: The SSID of the current vif. Valid in AP and IBSS mode.
>>>    * @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: TX power in dBm.  -1 means not configured.
>>>    * @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
>>> diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
>>> index b0c2df6e22c5..49fcf9d80f85 100644
>>> --- a/net/mac80211/iface.c
>>> +++ b/net/mac80211/iface.c
>>> @@ -1459,6 +1459,7 @@ static void ieee80211_setup_sdata(struct ieee80211_sub_if_data *sdata,
>>>       sdata->control_port_no_encrypt = false;
>>>       sdata->encrypt_headroom = IEEE80211_ENCRYPT_HEADROOM;
>>>       sdata->vif.bss_conf.idle = true;
>>> +     sdata->vif.bss_conf.txpower = -1; /* unset */
>>>
>>>       sdata->noack_map = 0;
>>>
>>> diff --git a/net/mac80211/main.c b/net/mac80211/main.c
>>> index a148509a88bc..2f53188851ee 100644
>>> --- a/net/mac80211/main.c
>>> +++ b/net/mac80211/main.c
>>> @@ -145,6 +145,8 @@ static u32 ieee80211_hw_conf_chan(struct ieee80211_local *local)
>>>                       continue;
>>>               if (sdata->vif.type == NL80211_IFTYPE_AP_VLAN)
>>>                       continue;
>>> +             if (sdata->vif.bss_conf.txpower < 0)
>>> +                     continue;
>>>               power = min(power, sdata->vif.bss_conf.txpower);
>>>       }
>>>       rcu_read_unlock();
>>>
>>
>>
>> --
>> Ben Greear <greearb@candelatech.com>
>> Candela Technologies Inc  http://www.candelatech.com
>>
>

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: [RFC] mac80211: Fix setting txpower to zero.

[Johannes Berg]

On Fri, 2019-12-13 at 15:03 -0800, greearb@candelatech.com wrote:
> 
> So, instead initialize the txpower to -1 in mac80211, and let drivers know that
> means the power has not been set and so should be ignored.

Technically (or maybe just physically?), even -1 is a sort of valid TX
power.

I know all of this is pretty messed up, but wouldn't it make more sense
to go with some kind of tx_power_valid bit, or perhaps something that
certainly will never make sense like MIN_INT instead of -1?

johannes

Re: [RFC] mac80211: Fix setting txpower to zero.

[Justin Capella]

So this may be silly to point out but I noticed the value is
multiplied by 2 later in txpower setup, which introduces the
possibility that of overflow/wrapping. min(power_limit, int_min) would
maybe run into trouble there.

Since the behavior is to assume 2/5g tx power, I think it would be
nice to set to those values (limits), then it would display sanely.
But I don't have a good grasp on this stuff, I imagine regdom plays a
role someplace too

On Mon, Dec 16, 2019 at 11:58 PM Johannes Berg
<johannes@sipsolutions.net> wrote:
>
> On Fri, 2019-12-13 at 15:03 -0800, greearb@candelatech.com wrote:
> >
> > So, instead initialize the txpower to -1 in mac80211, and let drivers know that
> > means the power has not been set and so should be ignored.
>
> Technically (or maybe just physically?), even -1 is a sort of valid TX
> power.
>
> I know all of this is pretty messed up, but wouldn't it make more sense
> to go with some kind of tx_power_valid bit, or perhaps something that
> certainly will never make sense like MIN_INT instead of -1?
>
> johannes
>

Re: [RFC] mac80211: Fix setting txpower to zero.

[Ben Greear]

On 12/16/2019 11:57 PM, Johannes Berg wrote:
> On Fri, 2019-12-13 at 15:03 -0800, greearb@candelatech.com wrote:
>>
>> So, instead initialize the txpower to -1 in mac80211, and let drivers know that
>> means the power has not been set and so should be ignored.
>
> Technically (or maybe just physically?), even -1 is a sort of valid TX
> power.
>
> I know all of this is pretty messed up, but wouldn't it make more sense
> to go with some kind of tx_power_valid bit, or perhaps something that
> certainly will never make sense like MIN_INT instead of -1?

I'm fine with using MIN_INT instead of -1 as the 'not-set' special value.

Certainly -1 dbm txpower can be legit, though not on the chipsets I am familiar
with as far as I can tell.

I'll redo the patch with MIN_INT later today.  I think that will be a lot less
change than adding a new flag that needs to be propagated to the drivers and stored
by drivers and such.

Thanks,
Ben

>
> johannes
>

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com