| Message ID | 20190320103723.27228-1-sven@narfation.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | b037b107565f329e737ec9ffdb121477a07245b6 |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | ath9k: Differentiate between max combined and per chain power | expand |
On 20/03/2019, Sven Eckelmann <sven@narfation.org> wrote: > This also means that a device with multiple tx chains is even limited > further: > > * 1 chain: 31 dBm per chain > * 2 chains: 28 dBm per chain > * 3 chains: 26 dBm per chain > This is flawed theory and should be reverted instead of making further complications. Simplest way would be to remove POWER_CORRECTION_FOR_TWO_CHAIN and POWER_CORRECTION_FOR_THREE_CHAIN; from driver
On Wednesday, 20 March 2019 14:49:57 CET Tom Psyborg wrote: > On 20/03/2019, Sven Eckelmann <sven@narfation.org> wrote: > > > This also means that a device with multiple tx chains is even limited > > further: > > > > * 1 chain: 31 dBm per chain > > * 2 chains: 28 dBm per chain > > * 3 chains: 26 dBm per chain > > > > This is flawed theory and should be reverted instead of making further > complications. Simplest way would be to remove > POWER_CORRECTION_FOR_TWO_CHAIN and POWER_CORRECTION_FOR_THREE_CHAIN; > from driver You can of course propose a patch for that. But I will not participate in the discussion how EIRP should be measured (per antenna/per system, on max point/ via integral over all positions, ...). And you should also discuss it with QCA to find out whether they will also remove the power correction from their closed source firmware. At least it would also affect ath10k... maybe even their other drivers with closed source components. And your change would also not completely supersede my change. You forget that the antennagain reduction is also done by scaled ath9k_hw_get_scaled_power. Kind regards, Sven
Sven Eckelmann <sven@narfation.org> wrote: > The ath9k driver uses as maximum allowed txpower the constant > MAX_RATE_POWER. It is used to set a maximum txpower limit for the PHY > (which is combined txpower) and also the maximum txpower for per chain > rates. Its value 63 is derived from the maximum number the registers can > store for the per chain txpower. > > The max txpower a user can set because of this is 31 dBm (floor(63 / 2)). > This also means that a device with multiple tx chains is even limited > further: > > * 1 chain: 31 dBm per chain > * 2 chains: 28 dBm per chain > * 3 chains: 26 dBm per chain > > This combined txpower limit of 31 dBm becomes even more problematic when > some extra antenna gain is set in the EEPROM. A high power device is then > no longer able to reach its potential limits. > > Instead the code dealing with the combined txpower must use a higher limit > than 63 and only the code dealing with the per chain txpower have to use > the limit of 63. Since the antenna gain can be quite large and 8 bit > variables are often used in ath9k for txpower, a large, divisible by two > number like 254 is a good choice for this new limit. > > Signed-off-by: Sven Eckelmann <sven@narfation.org> > Signed-off-by: Kalle Valo <kvalo@codeaurora.org> Patch applied to ath-next branch of ath.git, thanks. b037b107565f ath9k: Differentiate between max combined and per chain power
diff --git a/drivers/net/wireless/ath/ath9k/eeprom.c b/drivers/net/wireless/ath/ath9k/eeprom.c index 6fbd5559c0c0..c22d457dbc54 100644 --- a/drivers/net/wireless/ath/ath9k/eeprom.c +++ b/drivers/net/wireless/ath/ath9k/eeprom.c @@ -428,7 +428,7 @@ u16 ath9k_hw_get_scaled_power(struct ath_hw *ah, u16 power_limit, else power_limit = 0; - return power_limit; + return min_t(u16, power_limit, MAX_RATE_POWER); } void ath9k_hw_update_regulatory_maxpower(struct ath_hw *ah) diff --git a/drivers/net/wireless/ath/ath9k/eeprom_4k.c b/drivers/net/wireless/ath/ath9k/eeprom_4k.c index b8c0a08066a0..e8c2cc03be0c 100644 --- a/drivers/net/wireless/ath/ath9k/eeprom_4k.c +++ b/drivers/net/wireless/ath/ath9k/eeprom_4k.c @@ -424,6 +424,7 @@ static void ath9k_hw_set_4k_power_per_rate_table(struct ath_hw *ah, ath9k_hw_get_channel_centers(ah, chan, ¢ers); scaledPower = powerLimit - antenna_reduction; + scaledPower = min_t(u16, scaledPower, MAX_RATE_POWER); numCtlModes = ARRAY_SIZE(ctlModesFor11g) - SUB_NUM_CTL_MODES_AT_2G_40; pCtlMode = ctlModesFor11g; diff --git a/drivers/net/wireless/ath/ath9k/hw.c b/drivers/net/wireless/ath/ath9k/hw.c index 8581d917635a..f47322869bc3 100644 --- a/drivers/net/wireless/ath/ath9k/hw.c +++ b/drivers/net/wireless/ath/ath9k/hw.c @@ -446,7 +446,7 @@ static void ath9k_hw_init_defaults(struct ath_hw *ah) struct ath_regulatory *regulatory = ath9k_hw_regulatory(ah); regulatory->country_code = CTRY_DEFAULT; - regulatory->power_limit = MAX_RATE_POWER; + regulatory->power_limit = MAX_COMBINED_POWER; ah->hw_version.magic = AR5416_MAGIC; ah->hw_version.subvendorid = 0; @@ -2952,7 +2952,7 @@ void ath9k_hw_apply_txpower(struct ath_hw *ah, struct ath9k_channel *chan, ctl = ath9k_regd_get_ctl(reg, chan); channel = chan->chan; - chan_pwr = min_t(int, channel->max_power * 2, MAX_RATE_POWER); + chan_pwr = min_t(int, channel->max_power * 2, MAX_COMBINED_POWER); new_pwr = min_t(int, chan_pwr, reg->power_limit); ah->eep_ops->set_txpower(ah, chan, ctl, @@ -2965,9 +2965,9 @@ void ath9k_hw_set_txpowerlimit(struct ath_hw *ah, u32 limit, bool test) struct ath9k_channel *chan = ah->curchan; struct ieee80211_channel *channel = chan->chan; - reg->power_limit = min_t(u32, limit, MAX_RATE_POWER); + reg->power_limit = min_t(u32, limit, MAX_COMBINED_POWER); if (test) - channel->max_power = MAX_RATE_POWER / 2; + channel->max_power = MAX_COMBINED_POWER / 2; ath9k_hw_apply_txpower(ah, chan, test); diff --git a/drivers/net/wireless/ath/ath9k/hw.h b/drivers/net/wireless/ath/ath9k/hw.h index 68956cdc8c9a..2e4489700a85 100644 --- a/drivers/net/wireless/ath/ath9k/hw.h +++ b/drivers/net/wireless/ath/ath9k/hw.h @@ -173,6 +173,7 @@ #define ATH9K_NUM_QUEUES 10 #define MAX_RATE_POWER 63 +#define MAX_COMBINED_POWER 254 /* 128 dBm, chosen to fit in u8 */ #define AH_WAIT_TIMEOUT 100000 /* (us) */ #define AH_TSF_WRITE_TIMEOUT 100 /* (us) */ #define AH_TIME_QUANTUM 10 diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c index 98141b699c88..043801f773b5 100644 --- a/drivers/net/wireless/ath/ath9k/init.c +++ b/drivers/net/wireless/ath/ath9k/init.c @@ -805,7 +805,7 @@ static void ath9k_init_band_txpower(struct ath_softc *sc, int band) ah->curchan = &ah->channels[chan->hw_value]; cfg80211_chandef_create(&chandef, chan, NL80211_CHAN_HT20); ath9k_cmn_get_channel(sc->hw, ah, &chandef); - ath9k_hw_set_txpowerlimit(ah, MAX_RATE_POWER, true); + ath9k_hw_set_txpowerlimit(ah, MAX_COMBINED_POWER, true); } }
The ath9k driver uses as maximum allowed txpower the constant MAX_RATE_POWER. It is used to set a maximum txpower limit for the PHY (which is combined txpower) and also the maximum txpower for per chain rates. Its value 63 is derived from the maximum number the registers can store for the per chain txpower. The max txpower a user can set because of this is 31 dBm (floor(63 / 2)). This also means that a device with multiple tx chains is even limited further: * 1 chain: 31 dBm per chain * 2 chains: 28 dBm per chain * 3 chains: 26 dBm per chain This combined txpower limit of 31 dBm becomes even more problematic when some extra antenna gain is set in the EEPROM. A high power device is then no longer able to reach its potential limits. Instead the code dealing with the combined txpower must use a higher limit than 63 and only the code dealing with the per chain txpower have to use the limit of 63. Since the antenna gain can be quite large and 8 bit variables are often used in ath9k for txpower, a large, divisible by two number like 254 is a good choice for this new limit. Signed-off-by: Sven Eckelmann <sven@narfation.org> --- drivers/net/wireless/ath/ath9k/eeprom.c | 2 +- drivers/net/wireless/ath/ath9k/eeprom_4k.c | 1 + drivers/net/wireless/ath/ath9k/hw.c | 8 ++++---- drivers/net/wireless/ath/ath9k/hw.h | 1 + drivers/net/wireless/ath/ath9k/init.c | 2 +- 5 files changed, 8 insertions(+), 6 deletions(-)