Signed-off-by: Felix Fietkau <[email protected]>
---
drivers/net/wireless/ath/ath9k/beacon.c | 10 +++++-----
drivers/net/wireless/ath/ath9k/gpio.c | 4 ++--
drivers/net/wireless/ath/ath9k/mac.c | 6 +++---
drivers/net/wireless/ath/ath9k/mac.h | 2 +-
drivers/net/wireless/ath/ath9k/main.c | 10 +++++-----
drivers/net/wireless/ath/ath9k/recv.c | 2 +-
6 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/drivers/net/wireless/ath/ath9k/beacon.c b/drivers/net/wireless/ath/ath9k/beacon.c
index 9cdeaeb..a13cabb 100644
--- a/drivers/net/wireless/ath/ath9k/beacon.c
+++ b/drivers/net/wireless/ath/ath9k/beacon.c
@@ -515,7 +515,7 @@ static void ath_beacon_config_ap(struct ath_softc *sc,
sc->sc_flags |= SC_OP_TSF_RESET;
ath9k_beacon_init(sc, nexttbtt, intval);
sc->beacon.bmisscnt = 0;
- ath9k_hw_set_interrupts(ah, ah->imask);
+ ath9k_hw_set_interrupts(ah);
ath9k_hw_enable_interrupts(ah);
}
@@ -643,7 +643,7 @@ static void ath_beacon_config_sta(struct ath_softc *sc,
ath9k_hw_set_sta_beacon_timers(ah, &bs);
ah->imask |= ATH9K_INT_BMISS;
- ath9k_hw_set_interrupts(ah, ah->imask);
+ ath9k_hw_set_interrupts(ah);
ath9k_hw_enable_interrupts(ah);
}
@@ -679,7 +679,7 @@ static void ath_beacon_config_adhoc(struct ath_softc *sc,
ath9k_beacon_init(sc, nexttbtt, intval);
sc->beacon.bmisscnt = 0;
- ath9k_hw_set_interrupts(ah, ah->imask);
+ ath9k_hw_set_interrupts(ah);
ath9k_hw_enable_interrupts(ah);
}
@@ -821,11 +821,11 @@ void ath9k_set_beaconing_status(struct ath_softc *sc, bool status)
if (status) {
/* Re-enable beaconing */
ah->imask |= ATH9K_INT_SWBA;
- ath9k_hw_set_interrupts(ah, ah->imask);
+ ath9k_hw_set_interrupts(ah);
} else {
/* Disable SWBA interrupt */
ah->imask &= ~ATH9K_INT_SWBA;
- ath9k_hw_set_interrupts(ah, ah->imask);
+ ath9k_hw_set_interrupts(ah);
tasklet_kill(&sc->bcon_tasklet);
ath9k_hw_stop_dma_queue(ah, sc->beacon.beaconq);
}
diff --git a/drivers/net/wireless/ath/ath9k/gpio.c b/drivers/net/wireless/ath/ath9k/gpio.c
index afbf540..d357657 100644
--- a/drivers/net/wireless/ath/ath9k/gpio.c
+++ b/drivers/net/wireless/ath/ath9k/gpio.c
@@ -150,7 +150,7 @@ static void ath9k_gen_timer_start(struct ath_hw *ah,
if ((ah->imask & ATH9K_INT_GENTIMER) == 0) {
ath9k_hw_disable_interrupts(ah);
ah->imask |= ATH9K_INT_GENTIMER;
- ath9k_hw_set_interrupts(ah, ah->imask);
+ ath9k_hw_set_interrupts(ah);
ath9k_hw_enable_interrupts(ah);
}
}
@@ -165,7 +165,7 @@ static void ath9k_gen_timer_stop(struct ath_hw *ah, struct ath_gen_timer *timer)
if (timer_table->timer_mask.val == 0) {
ath9k_hw_disable_interrupts(ah);
ah->imask &= ~ATH9K_INT_GENTIMER;
- ath9k_hw_set_interrupts(ah, ah->imask);
+ ath9k_hw_set_interrupts(ah);
ath9k_hw_enable_interrupts(ah);
}
}
diff --git a/drivers/net/wireless/ath/ath9k/mac.c b/drivers/net/wireless/ath/ath9k/mac.c
index 22f23ea..835e81d 100644
--- a/drivers/net/wireless/ath/ath9k/mac.c
+++ b/drivers/net/wireless/ath/ath9k/mac.c
@@ -827,9 +827,9 @@ void ath9k_hw_enable_interrupts(struct ath_hw *ah)
}
EXPORT_SYMBOL(ath9k_hw_enable_interrupts);
-void ath9k_hw_set_interrupts(struct ath_hw *ah, enum ath9k_int ints)
+void ath9k_hw_set_interrupts(struct ath_hw *ah)
{
- enum ath9k_int omask = ah->imask;
+ enum ath9k_int ints = ah->imask;
u32 mask, mask2;
struct ath9k_hw_capabilities *pCap = &ah->caps;
struct ath_common *common = ath9k_hw_common(ah);
@@ -837,7 +837,7 @@ void ath9k_hw_set_interrupts(struct ath_hw *ah, enum ath9k_int ints)
if (!(ints & ATH9K_INT_GLOBAL))
ath9k_hw_disable_interrupts(ah);
- ath_dbg(common, ATH_DBG_INTERRUPT, "0x%x => 0x%x\n", omask, ints);
+ ath_dbg(common, ATH_DBG_INTERRUPT, "New interrupt mask 0x%x\n", ints);
mask = ints & ATH9K_INT_COMMON;
mask2 = 0;
diff --git a/drivers/net/wireless/ath/ath9k/mac.h b/drivers/net/wireless/ath/ath9k/mac.h
index 91c9654..c7cec9c 100644
--- a/drivers/net/wireless/ath/ath9k/mac.h
+++ b/drivers/net/wireless/ath/ath9k/mac.h
@@ -734,7 +734,7 @@ int ath9k_hw_beaconq_setup(struct ath_hw *ah);
/* Interrupt Handling */
bool ath9k_hw_intrpend(struct ath_hw *ah);
-void ath9k_hw_set_interrupts(struct ath_hw *ah, enum ath9k_int ints);
+void ath9k_hw_set_interrupts(struct ath_hw *ah);
void ath9k_hw_enable_interrupts(struct ath_hw *ah);
void ath9k_hw_disable_interrupts(struct ath_hw *ah);
diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index a16f539..f7cd461 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -273,7 +273,7 @@ static bool ath_complete_reset(struct ath_softc *sc, bool start)
ath9k_cmn_update_txpow(ah, sc->curtxpow,
sc->config.txpowlimit, &sc->curtxpow);
- ath9k_hw_set_interrupts(ah, ah->imask);
+ ath9k_hw_set_interrupts(ah);
ath9k_hw_enable_interrupts(ah);
if (!(sc->sc_flags & (SC_OP_OFFCHANNEL)) && start) {
@@ -823,7 +823,7 @@ irqreturn_t ath_isr(int irq, void *dev)
if (status & ATH9K_INT_RXEOL) {
ah->imask &= ~(ATH9K_INT_RXEOL | ATH9K_INT_RXORN);
- ath9k_hw_set_interrupts(ah, ah->imask);
+ ath9k_hw_set_interrupts(ah);
}
if (status & ATH9K_INT_MIB) {
@@ -1396,7 +1396,7 @@ static void ath9k_calculate_summary_state(struct ieee80211_hw *hw,
ah->imask &= ~ATH9K_INT_TSFOOR;
}
- ath9k_hw_set_interrupts(ah, ah->imask);
+ ath9k_hw_set_interrupts(ah);
/* Set up ANI */
if (iter_data.naps > 0) {
@@ -1571,7 +1571,7 @@ static void ath9k_enable_ps(struct ath_softc *sc)
if (!(ah->caps.hw_caps & ATH9K_HW_CAP_AUTOSLEEP)) {
if ((ah->imask & ATH9K_INT_TIM_TIMER) == 0) {
ah->imask |= ATH9K_INT_TIM_TIMER;
- ath9k_hw_set_interrupts(ah, ah->imask);
+ ath9k_hw_set_interrupts(ah);
}
ath9k_hw_setrxabort(ah, 1);
}
@@ -1591,7 +1591,7 @@ static void ath9k_disable_ps(struct ath_softc *sc)
PS_WAIT_FOR_TX_ACK);
if (ah->imask & ATH9K_INT_TIM_TIMER) {
ah->imask &= ~ATH9K_INT_TIM_TIMER;
- ath9k_hw_set_interrupts(ah, ah->imask);
+ ath9k_hw_set_interrupts(ah);
}
}
diff --git a/drivers/net/wireless/ath/ath9k/recv.c b/drivers/net/wireless/ath/ath9k/recv.c
index bcc0b22..bba93eb 100644
--- a/drivers/net/wireless/ath/ath9k/recv.c
+++ b/drivers/net/wireless/ath/ath9k/recv.c
@@ -1976,7 +1976,7 @@ requeue:
if (!(ah->imask & ATH9K_INT_RXEOL)) {
ah->imask |= (ATH9K_INT_RXEOL | ATH9K_INT_RXORN);
- ath9k_hw_set_interrupts(ah, ah->imask);
+ ath9k_hw_set_interrupts(ah);
}
return 0;
--
1.7.3.2
On Mon, Sep 19, 2011 at 10:38 AM, Felix Fietkau <[email protected]> wrote:
> Signed-off-by: Felix Fietkau <[email protected]>
Can you document on the patch commit log why you do remove this.
Luis
On Mon, Sep 19, 2011 at 1:50 PM, Felix Fietkau <[email protected]> wrote:
> On 2011-09-19 10:41 PM, Luis R. Rodriguez wrote:
>>
>> On Mon, Sep 19, 2011 at 10:38 AM, Felix Fietkau<[email protected]> wrote:
>>>
>>> @@ -986,21 +995,15 @@ static void
>>> ath9k_hw_set_def_power_per_rate_table(struct ath_hw *ah,
>>> struct ath9k_channel
>>> *chan,
>>> int16_t *ratesArray,
>>> u16 cfgCtl,
>>> - u16 AntennaReduction,
>>> - u16
>>> twiceMaxRegulatoryPower,
>>> + u16 antenna_reduction,
>>> u16 powerLimit)
>>> {
>>> #define REDUCE_SCALED_POWER_BY_TWO_CHAIN 6 /* 10*log10(2)*2 */
>>> #define REDUCE_SCALED_POWER_BY_THREE_CHAIN 9 /* 10*log10(3)*2 */
>>>
>>> - struct ath_regulatory *regulatory = ath9k_hw_regulatory(ah);
>>> struct ar5416_eeprom_def *pEepData =&ah->eeprom.def;
>>> u16 twiceMaxEdgePower = MAX_RATE_POWER;
>>> - static const u16 tpScaleReductionTable[5] =
>>> - { 0, 3, 6, 9, MAX_RATE_POWER };
>>> -
>>> int i;
>>> - int16_t twiceLargestAntenna;
>>> struct cal_ctl_data *rep;
>>> struct cal_target_power_leg targetPowerOfdm, targetPowerCck = {
>>> 0, { 0, 0, 0, 0}
>>> @@ -1012,7 +1015,7 @@ static void
>>> ath9k_hw_set_def_power_per_rate_table(struct ath_hw *ah,
>>> struct cal_target_power_ht targetPowerHt20, targetPowerHt40 = {
>>> 0, {0, 0, 0, 0}
>>> };
>>> - u16 scaledPower = 0, minCtlPower, maxRegAllowedPower;
>>> + u16 scaledPower = 0, minCtlPower;
>>> static const u16 ctlModesFor11a[] = {
>>> CTL_11A, CTL_5GHT20, CTL_11A_EXT, CTL_5GHT40
>>> };
>>
>> Although we do not use the reg->tp_scale I see no log which explains
>> that it will not, I'm reviewing TPC right now and we do need to
>> support it but its unclear to me yet if we are doing everything
>> correctly in mac80211 / cfg80211 for it. As far as I can see the
>> tp_scale stuff is used only if the default is not set, but as you
>> noted its always set to the default. I am trying to review the
>> internal code to see under what cases this changes but its hard to
>> review. Although I'd like to see this removed I'd prefer to treat this
>> separately from the cleanup you are doing, which I do find highly
>> valuable.
>
> Why keep it? All it does is subtract something from the max regulatory power
> level, so it does not make any sense to keep this crap duplicated in all the
> the eeprom files.
You're right that they do seem to use the same array values, but the
fact that its all common code is separate from the question of
removing it.
> If we ever do need it (which I doubt),
I suspect this is needed for APs that support DFS and since we do not
yet support DFS I do not think this is used. I am not 100% certain yet
but at least in my review in the last few minutes it seems the AP can
decide to change TPC constraints dynamically based on TPC reports from
the STAs. I suspec this is where this comes in to play.
> it needs to be added in a different place anyway.
If its card specific so I do not think we can move any of this
commonly into cfg80211 / mac80211.
One thing is to simply common code, another is to remove code which we
is not enabled right now, but may be later. I think we should treat
these separately.
Luis
On Mon, Sep 19, 2011 at 3:12 PM, Luis R. Rodriguez
<[email protected]> wrote:
> On Mon, Sep 19, 2011 at 3:04 PM, Felix Fietkau <[email protected]> wrote:
>> On 2011-09-19 11:54 PM, Luis R. Rodriguez wrote:
>>>
>>> On Mon, Sep 19, 2011 at 2:45 PM, Luis R. Rodriguez
>>> <[email protected]> wrote:
>>>>
>>>> On Mon, Sep 19, 2011 at 2:21 PM, Felix Fietkau<[email protected]> wrote:
>>>>>
>>>>> I looked at the other ath driver and I see no indication that it's
>>>>> related
>>>>> to DFS in any way.
>>>>
>>>> I have verified this just now as well, it seems it was only used to
>>>> support an ioctl to userspace to enable users to update a tpscale
>>>> value but I see no documentation about this. Next question is who in
>>>> usersapce sets this. I wonder if its done through userspace after
>>>> measuring some TPC reports from STAs.
>>>
>>> So this comes from supporting a "TR-098" specification, which seems to
>>> be the "Internet Gateway Device data model for the CPE WAN Management
>>> Protocol". I haven't yet been able to map this to the specification
>>> respective component:
>>>
>>> http://www.broadband-forum.org/technical/download/TR-098.pdf
>>
>> Interesting. That definitely supports my point that ath9k is the wrong place
>> for something like this to be. Let's just get rid of it.
>
> Yeah, I'm now convinced :) die code. But please do add some blurb
> about this tumor the code had.
In fact removing the tumor through a separate patch would be appreciated.
Luis
On 2011-09-19 8:18 PM, Luis R. Rodriguez wrote:
> On Mon, Sep 19, 2011 at 10:38 AM, Felix Fietkau<[email protected]> wrote:
>> Signed-off-by: Felix Fietkau<[email protected]>
>
> Can you document on the patch commit log why you do remove this.
Because it's unused
On Mon, Sep 19, 2011 at 11:29 AM, Felix Fietkau <[email protected]> wrote:
> On 2011-09-19 8:18 PM, Luis R. Rodriguez wrote:
>>
>> On Mon, Sep 19, 2011 at 10:38 AM, Felix Fietkau<[email protected]> wrote:
>>>
>>> Signed-off-by: Felix Fietkau<[email protected]>
>>
>> Can you document on the patch commit log why you do remove this.
>
> Because it's unused
Why is it unused?
Luis
On Mon, Sep 19, 2011 at 11:56 AM, Felix Fietkau <[email protected]> wrote:
> On 2011-09-19 8:43 PM, Luis R. Rodriguez wrote:
>>
>> On Mon, Sep 19, 2011 at 11:29 AM, Felix Fietkau<[email protected]> wrote:
>>>
>>> On 2011-09-19 8:18 PM, Luis R. Rodriguez wrote:
>>>>
>>>> On Mon, Sep 19, 2011 at 10:38 AM, Felix Fietkau<[email protected]>
>>>> wrote:
>>>>>
>>>>> Signed-off-by: Felix Fietkau<[email protected]>
>>>>
>>>> Can you document on the patch commit log why you do remove this.
>>>
>>> Because it's unused
>>
>> Why is it unused?
>
> Because the code that used to use it was removed a while ago in other
> patches.
And what did that do :) ?
Luis
It was previously used for current_rd_ext
Signed-off-by: Felix Fietkau <[email protected]>
---
drivers/net/wireless/ath/ath9k/ar9003_eeprom.c | 2 --
drivers/net/wireless/ath/ath9k/eeprom.h | 1 -
drivers/net/wireless/ath/ath9k/eeprom_4k.c | 2 --
drivers/net/wireless/ath/ath9k/eeprom_9287.c | 2 --
drivers/net/wireless/ath/ath9k/eeprom_def.c | 2 --
5 files changed, 0 insertions(+), 9 deletions(-)
diff --git a/drivers/net/wireless/ath/ath9k/ar9003_eeprom.c b/drivers/net/wireless/ath/ath9k/ar9003_eeprom.c
index d7a5ca7..bf08acc 100644
--- a/drivers/net/wireless/ath/ath9k/ar9003_eeprom.c
+++ b/drivers/net/wireless/ath/ath9k/ar9003_eeprom.c
@@ -2995,8 +2995,6 @@ static u32 ath9k_hw_ar9300_get_eeprom(struct ath_hw *ah,
return get_unaligned_be16(eep->macAddr + 4);
case EEP_REG_0:
return le16_to_cpu(pBase->regDmn[0]);
- case EEP_REG_1:
- return le16_to_cpu(pBase->regDmn[1]);
case EEP_OP_CAP:
return pBase->deviceCap;
case EEP_OP_MODE:
diff --git a/drivers/net/wireless/ath/ath9k/eeprom.h b/drivers/net/wireless/ath/ath9k/eeprom.h
index 085d3ba..291489b 100644
--- a/drivers/net/wireless/ath/ath9k/eeprom.h
+++ b/drivers/net/wireless/ath/ath9k/eeprom.h
@@ -225,7 +225,6 @@ enum eeprom_param {
EEP_MAC_MID,
EEP_MAC_LSW,
EEP_REG_0,
- EEP_REG_1,
EEP_OP_CAP,
EEP_OP_MODE,
EEP_RF_SILENT,
diff --git a/drivers/net/wireless/ath/ath9k/eeprom_4k.c b/drivers/net/wireless/ath/ath9k/eeprom_4k.c
index ab6811d..9a7520f 100644
--- a/drivers/net/wireless/ath/ath9k/eeprom_4k.c
+++ b/drivers/net/wireless/ath/ath9k/eeprom_4k.c
@@ -322,8 +322,6 @@ static u32 ath9k_hw_4k_get_eeprom(struct ath_hw *ah,
return get_unaligned_be16(pBase->macAddr + 4);
case EEP_REG_0:
return pBase->regDmn[0];
- case EEP_REG_1:
- return pBase->regDmn[1];
case EEP_OP_CAP:
return pBase->deviceCap;
case EEP_OP_MODE:
diff --git a/drivers/net/wireless/ath/ath9k/eeprom_9287.c b/drivers/net/wireless/ath/ath9k/eeprom_9287.c
index 90d771f..4f5c50a 100644
--- a/drivers/net/wireless/ath/ath9k/eeprom_9287.c
+++ b/drivers/net/wireless/ath/ath9k/eeprom_9287.c
@@ -308,8 +308,6 @@ static u32 ath9k_hw_ar9287_get_eeprom(struct ath_hw *ah,
return get_unaligned_be16(pBase->macAddr + 4);
case EEP_REG_0:
return pBase->regDmn[0];
- case EEP_REG_1:
- return pBase->regDmn[1];
case EEP_OP_CAP:
return pBase->deviceCap;
case EEP_OP_MODE:
diff --git a/drivers/net/wireless/ath/ath9k/eeprom_def.c b/drivers/net/wireless/ath/ath9k/eeprom_def.c
index e175e20..81e6296 100644
--- a/drivers/net/wireless/ath/ath9k/eeprom_def.c
+++ b/drivers/net/wireless/ath/ath9k/eeprom_def.c
@@ -415,8 +415,6 @@ static u32 ath9k_hw_def_get_eeprom(struct ath_hw *ah,
return get_unaligned_be16(pBase->macAddr + 4);
case EEP_REG_0:
return pBase->regDmn[0];
- case EEP_REG_1:
- return pBase->regDmn[1];
case EEP_OP_CAP:
return pBase->deviceCap;
case EEP_OP_MODE:
--
1.7.3.2
Signed-off-by: Felix Fietkau <[email protected]>
---
drivers/net/wireless/ath/ath.h | 1 -
drivers/net/wireless/ath/ath9k/hw.c | 5 -----
drivers/net/wireless/ath/carl9170/main.c | 1 -
3 files changed, 0 insertions(+), 7 deletions(-)
diff --git a/drivers/net/wireless/ath/ath.h b/drivers/net/wireless/ath/ath.h
index bb71b4f..908fdbc 100644
--- a/drivers/net/wireless/ath/ath.h
+++ b/drivers/net/wireless/ath/ath.h
@@ -72,7 +72,6 @@ struct ath_regulatory {
u16 country_code;
u16 max_power_level;
u16 current_rd;
- u16 current_rd_ext;
int16_t power_limit;
struct reg_dmn_pair_mapping *regpair;
};
diff --git a/drivers/net/wireless/ath/ath9k/hw.c b/drivers/net/wireless/ath/ath9k/hw.c
index 3255b5b..fbb69e4 100644
--- a/drivers/net/wireless/ath/ath9k/hw.c
+++ b/drivers/net/wireless/ath/ath9k/hw.c
@@ -2067,11 +2067,6 @@ int ath9k_hw_fill_cap_info(struct ath_hw *ah)
eeval = ah->eep_ops->get_eeprom(ah, EEP_REG_0);
regulatory->current_rd = eeval;
- eeval = ah->eep_ops->get_eeprom(ah, EEP_REG_1);
- if (AR_SREV_9285_12_OR_LATER(ah))
- eeval |= AR9285_RDEXT_DEFAULT;
- regulatory->current_rd_ext = eeval;
-
if (ah->opmode != NL80211_IFTYPE_AP &&
ah->hw_version.subvendorid == AR_SUBVENDOR_ID_NEW_A) {
if (regulatory->current_rd == 0x64 ||
diff --git a/drivers/net/wireless/ath/carl9170/main.c b/drivers/net/wireless/ath/carl9170/main.c
index af351ec..6786124 100644
--- a/drivers/net/wireless/ath/carl9170/main.c
+++ b/drivers/net/wireless/ath/carl9170/main.c
@@ -1894,7 +1894,6 @@ static int carl9170_parse_eeprom(struct ar9170 *ar)
ar->hw->channel_change_time = 80 * 1000;
regulatory->current_rd = le16_to_cpu(ar->eeprom.reg_domain[0]);
- regulatory->current_rd_ext = le16_to_cpu(ar->eeprom.reg_domain[1]);
/* second part of wiphy init */
SET_IEEE80211_PERM_ADDR(ar->hw, ar->eeprom.mac_address);
--
1.7.3.2
On Mon, Sep 19, 2011 at 3:04 PM, Felix Fietkau <[email protected]> wrote:
> On 2011-09-19 11:54 PM, Luis R. Rodriguez wrote:
>>
>> On Mon, Sep 19, 2011 at 2:45 PM, Luis R. Rodriguez
>> <[email protected]> wrote:
>>>
>>> On Mon, Sep 19, 2011 at 2:21 PM, Felix Fietkau<[email protected]> wrote:
>>>>
>>>> I looked at the other ath driver and I see no indication that it's
>>>> related
>>>> to DFS in any way.
>>>
>>> I have verified this just now as well, it seems it was only used to
>>> support an ioctl to userspace to enable users to update a tpscale
>>> value but I see no documentation about this. Next question is who in
>>> usersapce sets this. I wonder if its done through userspace after
>>> measuring some TPC reports from STAs.
>>
>> So this comes from supporting a "TR-098" specification, which seems to
>> be the "Internet Gateway Device data model for the CPE WAN Management
>> Protocol". I haven't yet been able to map this to the specification
>> respective component:
>>
>> http://www.broadband-forum.org/technical/download/TR-098.pdf
>
> Interesting. That definitely supports my point that ath9k is the wrong place
> for something like this to be. Let's just get rid of it.
Yeah, I'm now convinced :) die code. But please do add some blurb
about this tumor the code had.
Luis
On 2011-09-20 12:13 AM, Luis R. Rodriguez wrote:
> On Mon, Sep 19, 2011 at 3:12 PM, Luis R. Rodriguez
> <[email protected]> wrote:
>> On Mon, Sep 19, 2011 at 3:04 PM, Felix Fietkau<[email protected]> wrote:
>>> On 2011-09-19 11:54 PM, Luis R. Rodriguez wrote:
>>>>
>>>> On Mon, Sep 19, 2011 at 2:45 PM, Luis R. Rodriguez
>>>> <[email protected]> wrote:
>>>>>
>>>>> On Mon, Sep 19, 2011 at 2:21 PM, Felix Fietkau<[email protected]> wrote:
>>>>>>
>>>>>> I looked at the other ath driver and I see no indication that it's
>>>>>> related
>>>>>> to DFS in any way.
>>>>>
>>>>> I have verified this just now as well, it seems it was only used to
>>>>> support an ioctl to userspace to enable users to update a tpscale
>>>>> value but I see no documentation about this. Next question is who in
>>>>> usersapce sets this. I wonder if its done through userspace after
>>>>> measuring some TPC reports from STAs.
>>>>
>>>> So this comes from supporting a "TR-098" specification, which seems to
>>>> be the "Internet Gateway Device data model for the CPE WAN Management
>>>> Protocol". I haven't yet been able to map this to the specification
>>>> respective component:
>>>>
>>>> http://www.broadband-forum.org/technical/download/TR-098.pdf
>>>
>>> Interesting. That definitely supports my point that ath9k is the wrong place
>>> for something like this to be. Let's just get rid of it.
>>
>> Yeah, I'm now convinced :) die code. But please do add some blurb
>> about this tumor the code had.
>
> In fact removing the tumor through a separate patch would be appreciated.
I don't think it needs a separate patch. It's dead code directly related
to the other things that I'm changing, and it does not add any
functional changes. I'll add a comment, though.
- Felix
On Mon, Sep 19, 2011 at 09:03:17PM +0200, Felix Fietkau wrote:
> On 2011-09-19 8:58 PM, Luis R. Rodriguez wrote:
> >On Mon, Sep 19, 2011 at 11:56 AM, Felix Fietkau<[email protected]> wrote:
> >> On 2011-09-19 8:43 PM, Luis R. Rodriguez wrote:
> >>>
> >>> On Mon, Sep 19, 2011 at 11:29 AM, Felix Fietkau<[email protected]> wrote:
> >>>>
> >>>> On 2011-09-19 8:18 PM, Luis R. Rodriguez wrote:
> >>>>>
> >>>>> On Mon, Sep 19, 2011 at 10:38 AM, Felix Fietkau<[email protected]>
> >>>>> wrote:
> >>>>>>
> >>>>>> Signed-off-by: Felix Fietkau<[email protected]>
> >>>>>
> >>>>> Can you document on the patch commit log why you do remove this.
> >>>>
> >>>> Because it's unused
> >>>
> >>> Why is it unused?
> >>
> >> Because the code that used to use it was removed a while ago in other
> >> patches.
> >
> >And what did that do :) ?
> It used to set some pointless flags that were never used for anything.
I think he is prodding you to repost with an updated changelog...
--
John W. Linville Someday the world will need a hero, and you
[email protected] might be all we have. Be ready.
On 2011-09-19 8:58 PM, Luis R. Rodriguez wrote:
> On Mon, Sep 19, 2011 at 11:56 AM, Felix Fietkau<[email protected]> wrote:
>> On 2011-09-19 8:43 PM, Luis R. Rodriguez wrote:
>>>
>>> On Mon, Sep 19, 2011 at 11:29 AM, Felix Fietkau<[email protected]> wrote:
>>>>
>>>> On 2011-09-19 8:18 PM, Luis R. Rodriguez wrote:
>>>>>
>>>>> On Mon, Sep 19, 2011 at 10:38 AM, Felix Fietkau<[email protected]>
>>>>> wrote:
>>>>>>
>>>>>> Signed-off-by: Felix Fietkau<[email protected]>
>>>>>
>>>>> Can you document on the patch commit log why you do remove this.
>>>>
>>>> Because it's unused
>>>
>>> Why is it unused?
>>
>> Because the code that used to use it was removed a while ago in other
>> patches.
>
> And what did that do :) ?
It used to set some pointless flags that were never used for anything.
- Felix
On 2011-09-19 10:41 PM, Luis R. Rodriguez wrote:
> On Mon, Sep 19, 2011 at 10:38 AM, Felix Fietkau<[email protected]> wrote:
>> @@ -986,21 +995,15 @@ static void ath9k_hw_set_def_power_per_rate_table(struct ath_hw *ah,
>> struct ath9k_channel *chan,
>> int16_t *ratesArray,
>> u16 cfgCtl,
>> - u16 AntennaReduction,
>> - u16 twiceMaxRegulatoryPower,
>> + u16 antenna_reduction,
>> u16 powerLimit)
>> {
>> #define REDUCE_SCALED_POWER_BY_TWO_CHAIN 6 /* 10*log10(2)*2 */
>> #define REDUCE_SCALED_POWER_BY_THREE_CHAIN 9 /* 10*log10(3)*2 */
>>
>> - struct ath_regulatory *regulatory = ath9k_hw_regulatory(ah);
>> struct ar5416_eeprom_def *pEepData =&ah->eeprom.def;
>> u16 twiceMaxEdgePower = MAX_RATE_POWER;
>> - static const u16 tpScaleReductionTable[5] =
>> - { 0, 3, 6, 9, MAX_RATE_POWER };
>> -
>> int i;
>> - int16_t twiceLargestAntenna;
>> struct cal_ctl_data *rep;
>> struct cal_target_power_leg targetPowerOfdm, targetPowerCck = {
>> 0, { 0, 0, 0, 0}
>> @@ -1012,7 +1015,7 @@ static void ath9k_hw_set_def_power_per_rate_table(struct ath_hw *ah,
>> struct cal_target_power_ht targetPowerHt20, targetPowerHt40 = {
>> 0, {0, 0, 0, 0}
>> };
>> - u16 scaledPower = 0, minCtlPower, maxRegAllowedPower;
>> + u16 scaledPower = 0, minCtlPower;
>> static const u16 ctlModesFor11a[] = {
>> CTL_11A, CTL_5GHT20, CTL_11A_EXT, CTL_5GHT40
>> };
>
> Although we do not use the reg->tp_scale I see no log which explains
> that it will not, I'm reviewing TPC right now and we do need to
> support it but its unclear to me yet if we are doing everything
> correctly in mac80211 / cfg80211 for it. As far as I can see the
> tp_scale stuff is used only if the default is not set, but as you
> noted its always set to the default. I am trying to review the
> internal code to see under what cases this changes but its hard to
> review. Although I'd like to see this removed I'd prefer to treat this
> separately from the cleanup you are doing, which I do find highly
> valuable.
Why keep it? All it does is subtract something from the max regulatory
power level, so it does not make any sense to keep this crap duplicated
in all the the eeprom files.
If we ever do need it (which I doubt), it needs to be added in a
different place anyway.
- Felix
On Mon, Sep 19, 2011 at 2:45 PM, Luis R. Rodriguez
<[email protected]> wrote:
> On Mon, Sep 19, 2011 at 2:21 PM, Felix Fietkau <[email protected]> wrote:
>> I looked at the other ath driver and I see no indication that it's related
>> to DFS in any way.
>
> I have verified this just now as well, it seems it was only used to
> support an ioctl to userspace to enable users to update a tpscale
> value but I see no documentation about this. Next question is who in
> usersapce sets this. I wonder if its done through userspace after
> measuring some TPC reports from STAs.
So this comes from supporting a "TR-098" specification, which seems to
be the "Internet Gateway Device data model for the CPE WAN Management
Protocol". I haven't yet been able to map this to the specification
respective component:
http://www.broadband-forum.org/technical/download/TR-098.pdf
Luis
On 2011-09-19 7:38 PM, Felix Fietkau wrote:
> The code for handling various restrictions concerning regulatory limits,
> antenna gain, etc. is very convoluted and duplicated across various
> EEPROM parsing implementations, making it hard to review.
>
> This patch partially cleans up the mess by unifying regulatory limit
> handling in one function and removing dead code for transmit power scaling.
> It also simplifies handling of antenna gain
>
> Signed-off-by: Felix Fietkau<[email protected]>
Hmm, don't merge this just yet, seems that there are still some issues
with tx power limits on some devices. Will send a v3 when I've confirmed
the fix.
- Felix
On Mon, Sep 19, 2011 at 2:21 PM, Felix Fietkau <[email protected]> wrote:
> On 2011-09-19 11:14 PM, Luis R. Rodriguez wrote:
>>
>> On Mon, Sep 19, 2011 at 1:50 PM, Felix Fietkau<[email protected]> wrote:
>>>
>>> On 2011-09-19 10:41 PM, Luis R. Rodriguez wrote:
>>>>
>>>> On Mon, Sep 19, 2011 at 10:38 AM, Felix Fietkau<[email protected]>
>>>> wrote:
>>>>>
>>>>> @@ -986,21 +995,15 @@ static void
>>>>> ath9k_hw_set_def_power_per_rate_table(struct ath_hw *ah,
>>>>> struct ath9k_channel
>>>>> *chan,
>>>>> int16_t *ratesArray,
>>>>> u16 cfgCtl,
>>>>> - u16
>>>>> AntennaReduction,
>>>>> - u16
>>>>> twiceMaxRegulatoryPower,
>>>>> + u16
>>>>> antenna_reduction,
>>>>> u16 powerLimit)
>>>>> {
>>>>> #define REDUCE_SCALED_POWER_BY_TWO_CHAIN 6 /* 10*log10(2)*2 */
>>>>> #define REDUCE_SCALED_POWER_BY_THREE_CHAIN 9 /* 10*log10(3)*2 */
>>>>>
>>>>> - struct ath_regulatory *regulatory = ath9k_hw_regulatory(ah);
>>>>> struct ar5416_eeprom_def *pEepData =&ah->eeprom.def;
>>>>> u16 twiceMaxEdgePower = MAX_RATE_POWER;
>>>>> - static const u16 tpScaleReductionTable[5] =
>>>>> - { 0, 3, 6, 9, MAX_RATE_POWER };
>>>>> -
>>>>> int i;
>>>>> - int16_t twiceLargestAntenna;
>>>>> struct cal_ctl_data *rep;
>>>>> struct cal_target_power_leg targetPowerOfdm, targetPowerCck =
>>>>> {
>>>>> 0, { 0, 0, 0, 0}
>>>>> @@ -1012,7 +1015,7 @@ static void
>>>>> ath9k_hw_set_def_power_per_rate_table(struct ath_hw *ah,
>>>>> struct cal_target_power_ht targetPowerHt20, targetPowerHt40 =
>>>>> {
>>>>> 0, {0, 0, 0, 0}
>>>>> };
>>>>> - u16 scaledPower = 0, minCtlPower, maxRegAllowedPower;
>>>>> + u16 scaledPower = 0, minCtlPower;
>>>>> static const u16 ctlModesFor11a[] = {
>>>>> CTL_11A, CTL_5GHT20, CTL_11A_EXT, CTL_5GHT40
>>>>> };
>>>>
>>>> Although we do not use the reg->tp_scale I see no log which explains
>>>> that it will not, I'm reviewing TPC right now and we do need to
>>>> support it but its unclear to me yet if we are doing everything
>>>> correctly in mac80211 / cfg80211 for it. As far as I can see the
>>>> tp_scale stuff is used only if the default is not set, but as you
>>>> noted its always set to the default. I am trying to review the
>>>> internal code to see under what cases this changes but its hard to
>>>> review. Although I'd like to see this removed I'd prefer to treat this
>>>> separately from the cleanup you are doing, which I do find highly
>>>> valuable.
>>>
>>> Why keep it? All it does is subtract something from the max regulatory
>>> power
>>> level, so it does not make any sense to keep this crap duplicated in all
>>> the
>>> the eeprom files.
>>
>> You're right that they do seem to use the same array values, but the
>> fact that its all common code is separate from the question of
>> removing it.
>
> Right now it's dead code. If we need it later, we should just rewrite it. I
> think carring forward old dead code just in case we might use it later is
> not a good idea.
I agree with if the code provides no value, but I do not feel you have
proven this.
>>> If we ever do need it (which I doubt),
>>
>> I suspect this is needed for APs that support DFS and since we do not
>> yet support DFS I do not think this is used. I am not 100% certain yet
>> but at least in my review in the last few minutes it seems the AP can
>> decide to change TPC constraints dynamically based on TPC reports from
>> the STAs. I suspec this is where this comes in to play.
>
> I looked at the other ath driver and I see no indication that it's related
> to DFS in any way.
I have verified this just now as well, it seems it was only used to
support an ioctl to userspace to enable users to update a tpscale
value but I see no documentation about this. Next question is who in
usersapce sets this. I wonder if its done through userspace after
measuring some TPC reports from STAs.
> Even if it is, it doesn't even belong in ath9k. Any form
> of tx power reduction can be handled by cfg80211/mac80211.
Depends, general hooks can surely go into cfg80211 / mac80211 but if
any decision is being made to adjust power due to some environmental
constraints this would likely require chipset specific knobs or
information.
>>> it needs to be added in a different place anyway.
>>
>> If its card specific so I do not think we can move any of this
>> commonly into cfg80211 / mac80211.
>
> It's not card specific in any way.
In code right now its all the same arrays, but does it mean it will
not differ from broadcom's cards?
>> One thing is to simply common code, another is to remove code which we
>> is not enabled right now, but may be later. I think we should treat
>> these separately.
>
> I don't think we'll ever enable this, and it's not significant enough to
> keep it around.
I am not yet convinced its insignificant. I'd like to better
understand first who sets this and why before removing it. If anything
at least a separate patch should deal with the removal.
Luis
T24gTW9uLCBTZXAgMTksIDIwMTEgYXQgMTA6MzggQU0sIEZlbGl4IEZpZXRrYXUgPG5iZEBvcGVu
d3J0Lm9yZz4gd3JvdGU6Cj4gQEAgLTk4NiwyMSArOTk1LDE1IEBAIHN0YXRpYyB2b2lkIGF0aDlr
X2h3X3NldF9kZWZfcG93ZXJfcGVyX3JhdGVfdGFibGUoc3RydWN0IGF0aF9odyAqYWgsCj4gwqAg
wqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAg
wqAgwqAgwqAgwqAgwqBzdHJ1Y3QgYXRoOWtfY2hhbm5lbCAqY2hhbiwKPiDCoCDCoCDCoCDCoCDC
oCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDC
oCDCoGludDE2X3QgKnJhdGVzQXJyYXksCj4gwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAg
wqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqB1MTYgY2ZnQ3RsLAo+
IC0gwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAg
wqAgwqAgwqAgwqAgwqAgwqAgdTE2IEFudGVubmFSZWR1Y3Rpb24sCj4gLSDCoCDCoCDCoCDCoCDC
oCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDC
oCB1MTYgdHdpY2VNYXhSZWd1bGF0b3J5UG93ZXIsCj4gKyDCoCDCoCDCoCDCoCDCoCDCoCDCoCDC
oCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCB1MTYgYW50
ZW5uYV9yZWR1Y3Rpb24sCj4gwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAg
wqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqB1MTYgcG93ZXJMaW1pdCkKPiDCoHsK
PiDCoCNkZWZpbmUgUkVEVUNFX1NDQUxFRF9QT1dFUl9CWV9UV09fQ0hBSU4gwqAgwqAgNiDCoC8q
IDEwKmxvZzEwKDIpKjIgKi8KPiDCoCNkZWZpbmUgUkVEVUNFX1NDQUxFRF9QT1dFUl9CWV9USFJF
RV9DSEFJTiDCoCA5IC8qIDEwKmxvZzEwKDMpKjIgKi8KPgo+IC0gwqAgwqAgwqAgc3RydWN0IGF0
aF9yZWd1bGF0b3J5ICpyZWd1bGF0b3J5ID0gYXRoOWtfaHdfcmVndWxhdG9yeShhaCk7Cj4gwqAg
wqAgwqAgwqBzdHJ1Y3QgYXI1NDE2X2VlcHJvbV9kZWYgKnBFZXBEYXRhID0gJmFoLT5lZXByb20u
ZGVmOwo+IMKgIMKgIMKgIMKgdTE2IHR3aWNlTWF4RWRnZVBvd2VyID0gTUFYX1JBVEVfUE9XRVI7
Cj4gLSDCoCDCoCDCoCBzdGF0aWMgY29uc3QgdTE2IHRwU2NhbGVSZWR1Y3Rpb25UYWJsZVs1XSA9
Cj4gLSDCoCDCoCDCoCDCoCDCoCDCoCDCoCB7IDAsIDMsIDYsIDksIE1BWF9SQVRFX1BPV0VSIH07
Cj4gLQo+IMKgIMKgIMKgIMKgaW50IGk7Cj4gLSDCoCDCoCDCoCBpbnQxNl90IHR3aWNlTGFyZ2Vz
dEFudGVubmE7Cj4gwqAgwqAgwqAgwqBzdHJ1Y3QgY2FsX2N0bF9kYXRhICpyZXA7Cj4gwqAgwqAg
wqAgwqBzdHJ1Y3QgY2FsX3RhcmdldF9wb3dlcl9sZWcgdGFyZ2V0UG93ZXJPZmRtLCB0YXJnZXRQ
b3dlckNjayA9IHsKPiDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoDAsIHsgMCwgMCwgMCwgMH0KPiBA
QCAtMTAxMiw3ICsxMDE1LDcgQEAgc3RhdGljIHZvaWQgYXRoOWtfaHdfc2V0X2RlZl9wb3dlcl9w
ZXJfcmF0ZV90YWJsZShzdHJ1Y3QgYXRoX2h3ICphaCwKPiDCoCDCoCDCoCDCoHN0cnVjdCBjYWxf
dGFyZ2V0X3Bvd2VyX2h0IHRhcmdldFBvd2VySHQyMCwgdGFyZ2V0UG93ZXJIdDQwID0gewo+IMKg
IMKgIMKgIMKgIMKgIMKgIMKgIMKgMCwgezAsIDAsIDAsIDB9Cj4gwqAgwqAgwqAgwqB9Owo+IC0g
wqAgwqAgwqAgdTE2IHNjYWxlZFBvd2VyID0gMCwgbWluQ3RsUG93ZXIsIG1heFJlZ0FsbG93ZWRQ
b3dlcjsKPiArIMKgIMKgIMKgIHUxNiBzY2FsZWRQb3dlciA9IDAsIG1pbkN0bFBvd2VyOwo+IMKg
IMKgIMKgIMKgc3RhdGljIGNvbnN0IHUxNiBjdGxNb2Rlc0ZvcjExYVtdID0gewo+IMKgIMKgIMKg
IMKgIMKgIMKgIMKgIMKgQ1RMXzExQSwgQ1RMXzVHSFQyMCwgQ1RMXzExQV9FWFQsIENUTF81R0hU
NDAKPiDCoCDCoCDCoCDCoH07CgpBbHRob3VnaCB3ZSBkbyBub3QgdXNlIHRoZSByZWctPnRwX3Nj
YWxlIEkgc2VlIG5vIGxvZyB3aGljaCBleHBsYWlucwp0aGF0IGl0IHdpbGwgbm90LCBJJ20gcmV2
aWV3aW5nIFRQQyByaWdodCBub3cgYW5kIHdlIGRvIG5lZWQgdG8Kc3VwcG9ydCBpdCBidXQgaXRz
IHVuY2xlYXIgdG8gbWUgeWV0IGlmIHdlIGFyZSBkb2luZyBldmVyeXRoaW5nCmNvcnJlY3RseSBp
biBtYWM4MDIxMSAvIGNmZzgwMjExIGZvciBpdC4gQXMgZmFyIGFzIEkgY2FuIHNlZSB0aGUKdHBf
c2NhbGUgc3R1ZmYgaXMgdXNlZCBvbmx5IGlmIHRoZSBkZWZhdWx0IGlzIG5vdCBzZXQsIGJ1dCBh
cyB5b3UKbm90ZWQgaXRzIGFsd2F5cyBzZXQgdG8gdGhlIGRlZmF1bHQuIEkgYW0gdHJ5aW5nIHRv
IHJldmlldyB0aGUKaW50ZXJuYWwgY29kZSB0byBzZWUgdW5kZXIgd2hhdCBjYXNlcyB0aGlzIGNo
YW5nZXMgYnV0IGl0cyBoYXJkIHRvCnJldmlldy4gQWx0aG91Z2ggSSdkIGxpa2UgdG8gc2VlIHRo
aXMgcmVtb3ZlZCBJJ2QgcHJlZmVyIHRvIHRyZWF0IHRoaXMKc2VwYXJhdGVseSBmcm9tIHRoZSBj
bGVhbnVwIHlvdSBhcmUgZG9pbmcsIHdoaWNoIEkgZG8gZmluZCBoaWdobHkKdmFsdWFibGUuCgog
IEx1aXMK
On 2011-09-19 11:14 PM, Luis R. Rodriguez wrote:
> On Mon, Sep 19, 2011 at 1:50 PM, Felix Fietkau<[email protected]> wrote:
>> On 2011-09-19 10:41 PM, Luis R. Rodriguez wrote:
>>>
>>> On Mon, Sep 19, 2011 at 10:38 AM, Felix Fietkau<[email protected]> wrote:
>>>>
>>>> @@ -986,21 +995,15 @@ static void
>>>> ath9k_hw_set_def_power_per_rate_table(struct ath_hw *ah,
>>>> struct ath9k_channel
>>>> *chan,
>>>> int16_t *ratesArray,
>>>> u16 cfgCtl,
>>>> - u16 AntennaReduction,
>>>> - u16
>>>> twiceMaxRegulatoryPower,
>>>> + u16 antenna_reduction,
>>>> u16 powerLimit)
>>>> {
>>>> #define REDUCE_SCALED_POWER_BY_TWO_CHAIN 6 /* 10*log10(2)*2 */
>>>> #define REDUCE_SCALED_POWER_BY_THREE_CHAIN 9 /* 10*log10(3)*2 */
>>>>
>>>> - struct ath_regulatory *regulatory = ath9k_hw_regulatory(ah);
>>>> struct ar5416_eeprom_def *pEepData =&ah->eeprom.def;
>>>> u16 twiceMaxEdgePower = MAX_RATE_POWER;
>>>> - static const u16 tpScaleReductionTable[5] =
>>>> - { 0, 3, 6, 9, MAX_RATE_POWER };
>>>> -
>>>> int i;
>>>> - int16_t twiceLargestAntenna;
>>>> struct cal_ctl_data *rep;
>>>> struct cal_target_power_leg targetPowerOfdm, targetPowerCck = {
>>>> 0, { 0, 0, 0, 0}
>>>> @@ -1012,7 +1015,7 @@ static void
>>>> ath9k_hw_set_def_power_per_rate_table(struct ath_hw *ah,
>>>> struct cal_target_power_ht targetPowerHt20, targetPowerHt40 = {
>>>> 0, {0, 0, 0, 0}
>>>> };
>>>> - u16 scaledPower = 0, minCtlPower, maxRegAllowedPower;
>>>> + u16 scaledPower = 0, minCtlPower;
>>>> static const u16 ctlModesFor11a[] = {
>>>> CTL_11A, CTL_5GHT20, CTL_11A_EXT, CTL_5GHT40
>>>> };
>>>
>>> Although we do not use the reg->tp_scale I see no log which explains
>>> that it will not, I'm reviewing TPC right now and we do need to
>>> support it but its unclear to me yet if we are doing everything
>>> correctly in mac80211 / cfg80211 for it. As far as I can see the
>>> tp_scale stuff is used only if the default is not set, but as you
>>> noted its always set to the default. I am trying to review the
>>> internal code to see under what cases this changes but its hard to
>>> review. Although I'd like to see this removed I'd prefer to treat this
>>> separately from the cleanup you are doing, which I do find highly
>>> valuable.
>>
>> Why keep it? All it does is subtract something from the max regulatory power
>> level, so it does not make any sense to keep this crap duplicated in all the
>> the eeprom files.
>
> You're right that they do seem to use the same array values, but the
> fact that its all common code is separate from the question of
> removing it.
Right now it's dead code. If we need it later, we should just rewrite
it. I think carring forward old dead code just in case we might use it
later is not a good idea.
>> If we ever do need it (which I doubt),
>
> I suspect this is needed for APs that support DFS and since we do not
> yet support DFS I do not think this is used. I am not 100% certain yet
> but at least in my review in the last few minutes it seems the AP can
> decide to change TPC constraints dynamically based on TPC reports from
> the STAs. I suspec this is where this comes in to play.
I looked at the other ath driver and I see no indication that it's
related to DFS in any way. Even if it is, it doesn't even belong in
ath9k. Any form of tx power reduction can be handled by cfg80211/mac80211.
>> it needs to be added in a different place anyway.
>
> If its card specific so I do not think we can move any of this
> commonly into cfg80211 / mac80211.
It's not card specific in any way.
> One thing is to simply common code, another is to remove code which we
> is not enabled right now, but may be later. I think we should treat
> these separately.
I don't think we'll ever enable this, and it's not significant enough to
keep it around.
- Felix
On 2011-09-19 8:43 PM, Luis R. Rodriguez wrote:
> On Mon, Sep 19, 2011 at 11:29 AM, Felix Fietkau<[email protected]> wrote:
>> On 2011-09-19 8:18 PM, Luis R. Rodriguez wrote:
>>>
>>> On Mon, Sep 19, 2011 at 10:38 AM, Felix Fietkau<[email protected]> wrote:
>>>>
>>>> Signed-off-by: Felix Fietkau<[email protected]>
>>>
>>> Can you document on the patch commit log why you do remove this.
>>
>> Because it's unused
>
> Why is it unused?
Because the code that used to use it was removed a while ago in other
patches.
- Felix
The code for handling various restrictions concerning regulatory limits,
antenna gain, etc. is very convoluted and duplicated across various
EEPROM parsing implementations, making it hard to review.
This patch partially cleans up the mess by unifying regulatory limit
handling in one function and removing dead code for transmit power scaling.
It also simplifies handling of antenna gain
Signed-off-by: Felix Fietkau <[email protected]>
---
drivers/net/wireless/ath/ath.h | 1 -
drivers/net/wireless/ath/ath9k/ar5008_phy.c | 11 +----
drivers/net/wireless/ath/ath9k/ar9003_eeprom.c | 39 +++------------
drivers/net/wireless/ath/ath9k/ar9003_paprd.c | 9 +---
drivers/net/wireless/ath/ath9k/ar9003_phy.c | 11 +----
drivers/net/wireless/ath/ath9k/common.c | 6 ++-
drivers/net/wireless/ath/ath9k/eeprom.h | 7 ++-
drivers/net/wireless/ath/ath9k/eeprom_4k.c | 27 ++--------
drivers/net/wireless/ath/ath9k/eeprom_9287.c | 33 ++----------
drivers/net/wireless/ath/ath9k/eeprom_def.c | 43 +++++------------
drivers/net/wireless/ath/ath9k/hw.c | 63 ++++++++++++++++--------
drivers/net/wireless/ath/ath9k/hw.h | 9 +---
drivers/net/wireless/ath/ath9k/init.c | 2 -
13 files changed, 85 insertions(+), 176 deletions(-)
diff --git a/drivers/net/wireless/ath/ath.h b/drivers/net/wireless/ath/ath.h
index 4ed7f24..bb71b4f 100644
--- a/drivers/net/wireless/ath/ath.h
+++ b/drivers/net/wireless/ath/ath.h
@@ -71,7 +71,6 @@ struct ath_regulatory {
char alpha2[2];
u16 country_code;
u16 max_power_level;
- u32 tp_scale;
u16 current_rd;
u16 current_rd_ext;
int16_t power_limit;
diff --git a/drivers/net/wireless/ath/ath9k/ar5008_phy.c b/drivers/net/wireless/ath/ath9k/ar5008_phy.c
index 0a749c8..f199e9e 100644
--- a/drivers/net/wireless/ath/ath9k/ar5008_phy.c
+++ b/drivers/net/wireless/ath/ath9k/ar5008_phy.c
@@ -763,10 +763,8 @@ static void ar5008_hw_set_channel_regs(struct ath_hw *ah,
static int ar5008_hw_process_ini(struct ath_hw *ah,
struct ath9k_channel *chan)
{
- struct ath_regulatory *regulatory = ath9k_hw_regulatory(ah);
struct ath_common *common = ath9k_hw_common(ah);
int i, regWrites = 0;
- struct ieee80211_channel *channel = chan->chan;
u32 modesIndex, freqIndex;
switch (chan->chanmode) {
@@ -903,14 +901,7 @@ static int ar5008_hw_process_ini(struct ath_hw *ah,
ar5008_hw_set_channel_regs(ah, chan);
ar5008_hw_init_chain_masks(ah);
ath9k_olc_init(ah);
-
- /* Set TX power */
- ah->eep_ops->set_txpower(ah, chan,
- ath9k_regd_get_ctl(regulatory, chan),
- channel->max_antenna_gain * 2,
- channel->max_power * 2,
- min((u32) MAX_RATE_POWER,
- (u32) regulatory->power_limit), false);
+ ath9k_hw_apply_txpower(ah, chan);
/* Write analog registers */
if (!ath9k_hw_set_rf_regs(ah, chan, freqIndex)) {
diff --git a/drivers/net/wireless/ath/ath9k/ar9003_eeprom.c b/drivers/net/wireless/ath/ath9k/ar9003_eeprom.c
index 51398f0..d7a5ca7 100644
--- a/drivers/net/wireless/ath/ath9k/ar9003_eeprom.c
+++ b/drivers/net/wireless/ath/ath9k/ar9003_eeprom.c
@@ -3021,6 +3021,10 @@ static u32 ath9k_hw_ar9300_get_eeprom(struct ath_hw *ah,
return (pBase->miscConfiguration >> 0x3) & 0x1;
case EEP_ANT_DIV_CTL1:
return eep->base_ext1.ant_div_control;
+ case EEP_ANTENNA_GAIN_5G:
+ return eep->modalHeader5G.antennaGain;
+ case EEP_ANTENNA_GAIN_2G:
+ return eep->modalHeader2G.antennaGain;
default:
return 0;
}
@@ -4764,20 +4768,14 @@ static u16 ar9003_hw_get_max_edge_power(struct ar9300_eeprom *eep,
static void ar9003_hw_set_power_per_rate_table(struct ath_hw *ah,
struct ath9k_channel *chan,
u8 *pPwrArray, u16 cfgCtl,
- u8 twiceAntennaReduction,
- u8 twiceMaxRegulatoryPower,
+ u8 antenna_reduction,
u16 powerLimit)
{
- struct ath_regulatory *regulatory = ath9k_hw_regulatory(ah);
struct ath_common *common = ath9k_hw_common(ah);
struct ar9300_eeprom *pEepData = &ah->eeprom.ar9300_eep;
u16 twiceMaxEdgePower = MAX_RATE_POWER;
- static const u16 tpScaleReductionTable[5] = {
- 0, 3, 6, 9, MAX_RATE_POWER
- };
int i;
- int16_t twiceLargestAntenna;
- u16 scaledPower = 0, minCtlPower, maxRegAllowedPower;
+ u16 scaledPower = 0, minCtlPower;
static const u16 ctlModesFor11a[] = {
CTL_11A, CTL_5GHT20, CTL_11A_EXT, CTL_5GHT40
};
@@ -4795,28 +4793,7 @@ static void ar9003_hw_set_power_per_rate_table(struct ath_hw *ah,
bool is2ghz = IS_CHAN_2GHZ(chan);
ath9k_hw_get_channel_centers(ah, chan, ¢ers);
-
- /* Compute TxPower reduction due to Antenna Gain */
- if (is2ghz)
- twiceLargestAntenna = pEepData->modalHeader2G.antennaGain;
- else
- twiceLargestAntenna = pEepData->modalHeader5G.antennaGain;
-
- twiceLargestAntenna = (int16_t)min((twiceAntennaReduction) -
- twiceLargestAntenna, 0);
-
- /*
- * scaledPower is the minimum of the user input power level
- * and the regulatory allowed power level
- */
- maxRegAllowedPower = twiceMaxRegulatoryPower + twiceLargestAntenna;
-
- if (regulatory->tp_scale != ATH9K_TP_SCALE_MAX) {
- maxRegAllowedPower -=
- (tpScaleReductionTable[(regulatory->tp_scale)] * 2);
- }
-
- scaledPower = min(powerLimit, maxRegAllowedPower);
+ scaledPower = powerLimit - antenna_reduction;
/*
* Reduce scaled Power by number of chains active to get
@@ -5003,7 +4980,6 @@ static inline u8 mcsidx_to_tgtpwridx(unsigned int mcs_idx, u8 base_pwridx)
static void ath9k_hw_ar9300_set_txpower(struct ath_hw *ah,
struct ath9k_channel *chan, u16 cfgCtl,
u8 twiceAntennaReduction,
- u8 twiceMaxRegulatoryPower,
u8 powerLimit, bool test)
{
struct ath_regulatory *regulatory = ath9k_hw_regulatory(ah);
@@ -5056,7 +5032,6 @@ static void ath9k_hw_ar9300_set_txpower(struct ath_hw *ah,
ar9003_hw_set_power_per_rate_table(ah, chan,
targetPowerValT2, cfgCtl,
twiceAntennaReduction,
- twiceMaxRegulatoryPower,
powerLimit);
if (ah->eep_ops->get_eeprom(ah, EEP_PAPRD)) {
diff --git a/drivers/net/wireless/ath/ath9k/ar9003_paprd.c b/drivers/net/wireless/ath/ath9k/ar9003_paprd.c
index 609acb2..a1a08b3 100644
--- a/drivers/net/wireless/ath/ath9k/ar9003_paprd.c
+++ b/drivers/net/wireless/ath/ath9k/ar9003_paprd.c
@@ -19,7 +19,6 @@
void ar9003_paprd_enable(struct ath_hw *ah, bool val)
{
- struct ath_regulatory *regulatory = ath9k_hw_regulatory(ah);
struct ath9k_channel *chan = ah->curchan;
struct ar9300_eeprom *eep = &ah->eeprom.ar9300_eep;
@@ -54,13 +53,7 @@ void ar9003_paprd_enable(struct ath_hw *ah, bool val)
if (val) {
ah->paprd_table_write_done = true;
-
- ah->eep_ops->set_txpower(ah, chan,
- ath9k_regd_get_ctl(regulatory, chan),
- chan->chan->max_antenna_gain * 2,
- chan->chan->max_power * 2,
- min((u32) MAX_RATE_POWER,
- (u32) regulatory->power_limit), false);
+ ath9k_hw_apply_txpower(ah, chan);
}
REG_RMW_FIELD(ah, AR_PHY_PAPRD_CTRL0_B0,
diff --git a/drivers/net/wireless/ath/ath9k/ar9003_phy.c b/drivers/net/wireless/ath/ath9k/ar9003_phy.c
index 7db6e86..779f407 100644
--- a/drivers/net/wireless/ath/ath9k/ar9003_phy.c
+++ b/drivers/net/wireless/ath/ath9k/ar9003_phy.c
@@ -631,9 +631,7 @@ static void ar9003_hw_prog_ini(struct ath_hw *ah,
static int ar9003_hw_process_ini(struct ath_hw *ah,
struct ath9k_channel *chan)
{
- struct ath_regulatory *regulatory = ath9k_hw_regulatory(ah);
unsigned int regWrites = 0, i;
- struct ieee80211_channel *channel = chan->chan;
u32 modesIndex;
switch (chan->chanmode) {
@@ -693,14 +691,7 @@ static int ar9003_hw_process_ini(struct ath_hw *ah,
ar9003_hw_override_ini(ah);
ar9003_hw_set_channel_regs(ah, chan);
ar9003_hw_set_chain_masks(ah, ah->rxchainmask, ah->txchainmask);
-
- /* Set TX power */
- ah->eep_ops->set_txpower(ah, chan,
- ath9k_regd_get_ctl(regulatory, chan),
- channel->max_antenna_gain * 2,
- channel->max_power * 2,
- min((u32) MAX_RATE_POWER,
- (u32) regulatory->power_limit), false);
+ ath9k_hw_apply_txpower(ah, chan);
return 0;
}
diff --git a/drivers/net/wireless/ath/ath9k/common.c b/drivers/net/wireless/ath/ath9k/common.c
index dc705a2..905f1b3 100644
--- a/drivers/net/wireless/ath/ath9k/common.c
+++ b/drivers/net/wireless/ath/ath9k/common.c
@@ -161,10 +161,12 @@ EXPORT_SYMBOL(ath9k_cmn_count_streams);
void ath9k_cmn_update_txpow(struct ath_hw *ah, u16 cur_txpow,
u16 new_txpow, u16 *txpower)
{
- if (cur_txpow != new_txpow) {
+ struct ath_regulatory *reg = ath9k_hw_regulatory(ah);
+
+ if (reg->power_limit != new_txpow) {
ath9k_hw_set_txpowerlimit(ah, new_txpow, false);
/* read back in case value is clamped */
- *txpower = ath9k_hw_regulatory(ah)->power_limit;
+ *txpower = reg->max_power_level;
}
}
EXPORT_SYMBOL(ath9k_cmn_update_txpow);
diff --git a/drivers/net/wireless/ath/ath9k/eeprom.h b/drivers/net/wireless/ath/ath9k/eeprom.h
index a3c7d0c2..085d3ba 100644
--- a/drivers/net/wireless/ath/ath9k/eeprom.h
+++ b/drivers/net/wireless/ath/ath9k/eeprom.h
@@ -253,7 +253,9 @@ enum eeprom_param {
EEP_PAPRD,
EEP_MODAL_VER,
EEP_ANT_DIV_CTL1,
- EEP_CHAIN_MASK_REDUCE
+ EEP_CHAIN_MASK_REDUCE,
+ EEP_ANTENNA_GAIN_2G,
+ EEP_ANTENNA_GAIN_5G
};
enum ar5416_rates {
@@ -657,8 +659,7 @@ struct eeprom_ops {
void (*set_addac)(struct ath_hw *hw, struct ath9k_channel *chan);
void (*set_txpower)(struct ath_hw *hw, struct ath9k_channel *chan,
u16 cfgCtl, u8 twiceAntennaReduction,
- u8 twiceMaxRegulatoryPower, u8 powerLimit,
- bool test);
+ u8 powerLimit, bool test);
u16 (*get_spur_channel)(struct ath_hw *ah, u16 i, bool is2GHz);
};
diff --git a/drivers/net/wireless/ath/ath9k/eeprom_4k.c b/drivers/net/wireless/ath/ath9k/eeprom_4k.c
index 303560e..ab6811d 100644
--- a/drivers/net/wireless/ath/ath9k/eeprom_4k.c
+++ b/drivers/net/wireless/ath/ath9k/eeprom_4k.c
@@ -350,6 +350,8 @@ static u32 ath9k_hw_4k_get_eeprom(struct ath_hw *ah,
return pModal->antdiv_ctl1;
case EEP_TXGAIN_TYPE:
return pBase->txGainType;
+ case EEP_ANTENNA_GAIN_2G:
+ return pModal->antennaGainCh[0];
default:
return 0;
}
@@ -462,8 +464,7 @@ static void ath9k_hw_set_4k_power_per_rate_table(struct ath_hw *ah,
struct ath9k_channel *chan,
int16_t *ratesArray,
u16 cfgCtl,
- u16 AntennaReduction,
- u16 twiceMaxRegulatoryPower,
+ u16 antenna_reduction,
u16 powerLimit)
{
#define CMP_TEST_GRP \
@@ -472,20 +473,16 @@ static void ath9k_hw_set_4k_power_per_rate_table(struct ath_hw *ah,
|| (((cfgCtl & ~CTL_MODE_M) | (pCtlMode[ctlMode] & CTL_MODE_M)) == \
((pEepData->ctlIndex[i] & CTL_MODE_M) | SD_NO_CTL))
- struct ath_regulatory *regulatory = ath9k_hw_regulatory(ah);
int i;
- int16_t twiceLargestAntenna;
u16 twiceMinEdgePower;
u16 twiceMaxEdgePower = MAX_RATE_POWER;
- u16 scaledPower = 0, minCtlPower, maxRegAllowedPower;
+ u16 scaledPower = 0, minCtlPower;
u16 numCtlModes;
const u16 *pCtlMode;
u16 ctlMode, freq;
struct chan_centers centers;
struct cal_ctl_data_4k *rep;
struct ar5416_eeprom_4k *pEepData = &ah->eeprom.map4k;
- static const u16 tpScaleReductionTable[5] =
- { 0, 3, 6, 9, MAX_RATE_POWER };
struct cal_target_power_leg targetPowerOfdm, targetPowerCck = {
0, { 0, 0, 0, 0}
};
@@ -503,19 +500,7 @@ static void ath9k_hw_set_4k_power_per_rate_table(struct ath_hw *ah,
ath9k_hw_get_channel_centers(ah, chan, ¢ers);
- twiceLargestAntenna = pEepData->modalHeader.antennaGainCh[0];
- twiceLargestAntenna = (int16_t)min(AntennaReduction -
- twiceLargestAntenna, 0);
-
- maxRegAllowedPower = twiceMaxRegulatoryPower + twiceLargestAntenna;
- if (regulatory->tp_scale != ATH9K_TP_SCALE_MAX) {
- maxRegAllowedPower -=
- (tpScaleReductionTable[(regulatory->tp_scale)] * 2);
- }
-
- scaledPower = min(powerLimit, maxRegAllowedPower);
- scaledPower = max((u16)0, scaledPower);
-
+ scaledPower = powerLimit - antenna_reduction;
numCtlModes = ARRAY_SIZE(ctlModesFor11g) - SUB_NUM_CTL_MODES_AT_2G_40;
pCtlMode = ctlModesFor11g;
@@ -671,7 +656,6 @@ static void ath9k_hw_4k_set_txpower(struct ath_hw *ah,
struct ath9k_channel *chan,
u16 cfgCtl,
u8 twiceAntennaReduction,
- u8 twiceMaxRegulatoryPower,
u8 powerLimit, bool test)
{
struct ath_regulatory *regulatory = ath9k_hw_regulatory(ah);
@@ -691,7 +675,6 @@ static void ath9k_hw_4k_set_txpower(struct ath_hw *ah,
ath9k_hw_set_4k_power_per_rate_table(ah, chan,
&ratesArray[0], cfgCtl,
twiceAntennaReduction,
- twiceMaxRegulatoryPower,
powerLimit);
ath9k_hw_set_4k_power_cal_table(ah, chan);
diff --git a/drivers/net/wireless/ath/ath9k/eeprom_9287.c b/drivers/net/wireless/ath/ath9k/eeprom_9287.c
index 6698b72..90d771f 100644
--- a/drivers/net/wireless/ath/ath9k/eeprom_9287.c
+++ b/drivers/net/wireless/ath/ath9k/eeprom_9287.c
@@ -336,6 +336,9 @@ static u32 ath9k_hw_ar9287_get_eeprom(struct ath_hw *ah,
return pBase->tempSensSlopePalOn;
else
return 0;
+ case EEP_ANTENNA_GAIN_2G:
+ return max_t(u8, pModal->antennaGainCh[0],
+ pModal->antennaGainCh[1]);
default:
return 0;
}
@@ -554,8 +557,7 @@ static void ath9k_hw_set_ar9287_power_per_rate_table(struct ath_hw *ah,
struct ath9k_channel *chan,
int16_t *ratesArray,
u16 cfgCtl,
- u16 AntennaReduction,
- u16 twiceMaxRegulatoryPower,
+ u16 antenna_reduction,
u16 powerLimit)
{
#define CMP_CTL \
@@ -569,12 +571,8 @@ static void ath9k_hw_set_ar9287_power_per_rate_table(struct ath_hw *ah,
#define REDUCE_SCALED_POWER_BY_TWO_CHAIN 6
#define REDUCE_SCALED_POWER_BY_THREE_CHAIN 10
- struct ath_regulatory *regulatory = ath9k_hw_regulatory(ah);
u16 twiceMaxEdgePower = MAX_RATE_POWER;
- static const u16 tpScaleReductionTable[5] =
- { 0, 3, 6, 9, MAX_RATE_POWER };
int i;
- int16_t twiceLargestAntenna;
struct cal_ctl_data_ar9287 *rep;
struct cal_target_power_leg targetPowerOfdm = {0, {0, 0, 0, 0} },
targetPowerCck = {0, {0, 0, 0, 0} };
@@ -582,7 +580,7 @@ static void ath9k_hw_set_ar9287_power_per_rate_table(struct ath_hw *ah,
targetPowerCckExt = {0, {0, 0, 0, 0} };
struct cal_target_power_ht targetPowerHt20,
targetPowerHt40 = {0, {0, 0, 0, 0} };
- u16 scaledPower = 0, minCtlPower, maxRegAllowedPower;
+ u16 scaledPower = 0, minCtlPower;
static const u16 ctlModesFor11g[] = {
CTL_11B, CTL_11G, CTL_2GHT20,
CTL_11B_EXT, CTL_11G_EXT, CTL_2GHT40
@@ -597,24 +595,7 @@ static void ath9k_hw_set_ar9287_power_per_rate_table(struct ath_hw *ah,
tx_chainmask = ah->txchainmask;
ath9k_hw_get_channel_centers(ah, chan, ¢ers);
-
- /* Compute TxPower reduction due to Antenna Gain */
- twiceLargestAntenna = max(pEepData->modalHeader.antennaGainCh[0],
- pEepData->modalHeader.antennaGainCh[1]);
- twiceLargestAntenna = (int16_t)min((AntennaReduction) -
- twiceLargestAntenna, 0);
-
- /*
- * scaledPower is the minimum of the user input power level
- * and the regulatory allowed power level.
- */
- maxRegAllowedPower = twiceMaxRegulatoryPower + twiceLargestAntenna;
-
- if (regulatory->tp_scale != ATH9K_TP_SCALE_MAX)
- maxRegAllowedPower -=
- (tpScaleReductionTable[(regulatory->tp_scale)] * 2);
-
- scaledPower = min(powerLimit, maxRegAllowedPower);
+ scaledPower = powerLimit - antenna_reduction;
/*
* Reduce scaled Power by number of chains active
@@ -815,7 +796,6 @@ static void ath9k_hw_set_ar9287_power_per_rate_table(struct ath_hw *ah,
static void ath9k_hw_ar9287_set_txpower(struct ath_hw *ah,
struct ath9k_channel *chan, u16 cfgCtl,
u8 twiceAntennaReduction,
- u8 twiceMaxRegulatoryPower,
u8 powerLimit, bool test)
{
struct ath_regulatory *regulatory = ath9k_hw_regulatory(ah);
@@ -834,7 +814,6 @@ static void ath9k_hw_ar9287_set_txpower(struct ath_hw *ah,
ath9k_hw_set_ar9287_power_per_rate_table(ah, chan,
&ratesArray[0], cfgCtl,
twiceAntennaReduction,
- twiceMaxRegulatoryPower,
powerLimit);
ath9k_hw_set_ar9287_power_cal_table(ah, chan);
diff --git a/drivers/net/wireless/ath/ath9k/eeprom_def.c b/drivers/net/wireless/ath/ath9k/eeprom_def.c
index eda681f..e175e20 100644
--- a/drivers/net/wireless/ath/ath9k/eeprom_def.c
+++ b/drivers/net/wireless/ath/ath9k/eeprom_def.c
@@ -400,6 +400,7 @@ static u32 ath9k_hw_def_get_eeprom(struct ath_hw *ah,
struct ar5416_eeprom_def *eep = &ah->eeprom.def;
struct modal_eep_header *pModal = eep->modalHeader;
struct base_eep_header *pBase = &eep->baseEepHeader;
+ int band = 0;
switch (param) {
case EEP_NFTHRESH_5:
@@ -467,6 +468,14 @@ static u32 ath9k_hw_def_get_eeprom(struct ath_hw *ah,
return pBase->pwr_table_offset;
else
return AR5416_PWR_TABLE_OFFSET_DB;
+ case EEP_ANTENNA_GAIN_2G:
+ band = 1;
+ /* fall through */
+ case EEP_ANTENNA_GAIN_5G:
+ return max_t(u8, max_t(u8,
+ pModal[band].antennaGainCh[0],
+ pModal[band].antennaGainCh[1]),
+ pModal[band].antennaGainCh[2]);
default:
return 0;
}
@@ -986,21 +995,15 @@ static void ath9k_hw_set_def_power_per_rate_table(struct ath_hw *ah,
struct ath9k_channel *chan,
int16_t *ratesArray,
u16 cfgCtl,
- u16 AntennaReduction,
- u16 twiceMaxRegulatoryPower,
+ u16 antenna_reduction,
u16 powerLimit)
{
#define REDUCE_SCALED_POWER_BY_TWO_CHAIN 6 /* 10*log10(2)*2 */
#define REDUCE_SCALED_POWER_BY_THREE_CHAIN 9 /* 10*log10(3)*2 */
- struct ath_regulatory *regulatory = ath9k_hw_regulatory(ah);
struct ar5416_eeprom_def *pEepData = &ah->eeprom.def;
u16 twiceMaxEdgePower = MAX_RATE_POWER;
- static const u16 tpScaleReductionTable[5] =
- { 0, 3, 6, 9, MAX_RATE_POWER };
-
int i;
- int16_t twiceLargestAntenna;
struct cal_ctl_data *rep;
struct cal_target_power_leg targetPowerOfdm, targetPowerCck = {
0, { 0, 0, 0, 0}
@@ -1012,7 +1015,7 @@ static void ath9k_hw_set_def_power_per_rate_table(struct ath_hw *ah,
struct cal_target_power_ht targetPowerHt20, targetPowerHt40 = {
0, {0, 0, 0, 0}
};
- u16 scaledPower = 0, minCtlPower, maxRegAllowedPower;
+ u16 scaledPower = 0, minCtlPower;
static const u16 ctlModesFor11a[] = {
CTL_11A, CTL_5GHT20, CTL_11A_EXT, CTL_5GHT40
};
@@ -1031,27 +1034,7 @@ static void ath9k_hw_set_def_power_per_rate_table(struct ath_hw *ah,
ath9k_hw_get_channel_centers(ah, chan, ¢ers);
- twiceLargestAntenna = max(
- pEepData->modalHeader
- [IS_CHAN_2GHZ(chan)].antennaGainCh[0],
- pEepData->modalHeader
- [IS_CHAN_2GHZ(chan)].antennaGainCh[1]);
-
- twiceLargestAntenna = max((u8)twiceLargestAntenna,
- pEepData->modalHeader
- [IS_CHAN_2GHZ(chan)].antennaGainCh[2]);
-
- twiceLargestAntenna = (int16_t)min(AntennaReduction -
- twiceLargestAntenna, 0);
-
- maxRegAllowedPower = twiceMaxRegulatoryPower + twiceLargestAntenna;
-
- if (regulatory->tp_scale != ATH9K_TP_SCALE_MAX) {
- maxRegAllowedPower -=
- (tpScaleReductionTable[(regulatory->tp_scale)] * 2);
- }
-
- scaledPower = min(powerLimit, maxRegAllowedPower);
+ scaledPower = powerLimit - antenna_reduction;
switch (ar5416_get_ntxchains(tx_chainmask)) {
case 1:
@@ -1256,7 +1239,6 @@ static void ath9k_hw_def_set_txpower(struct ath_hw *ah,
struct ath9k_channel *chan,
u16 cfgCtl,
u8 twiceAntennaReduction,
- u8 twiceMaxRegulatoryPower,
u8 powerLimit, bool test)
{
#define RT_AR_DELTA(x) (ratesArray[x] - cck_ofdm_delta)
@@ -1278,7 +1260,6 @@ static void ath9k_hw_def_set_txpower(struct ath_hw *ah,
ath9k_hw_set_def_power_per_rate_table(ah, chan,
&ratesArray[0], cfgCtl,
twiceAntennaReduction,
- twiceMaxRegulatoryPower,
powerLimit);
ath9k_hw_set_def_power_cal_table(ah, chan);
diff --git a/drivers/net/wireless/ath/ath9k/hw.c b/drivers/net/wireless/ath/ath9k/hw.c
index f2de7ee..3255b5b 100644
--- a/drivers/net/wireless/ath/ath9k/hw.c
+++ b/drivers/net/wireless/ath/ath9k/hw.c
@@ -428,7 +428,6 @@ static void ath9k_hw_init_defaults(struct ath_hw *ah)
regulatory->country_code = CTRY_DEFAULT;
regulatory->power_limit = MAX_RATE_POWER;
- regulatory->tp_scale = ATH9K_TP_SCALE_MAX;
ah->hw_version.magic = AR5416_MAGIC;
ah->hw_version.subvendorid = 0;
@@ -1384,9 +1383,7 @@ static bool ath9k_hw_chip_reset(struct ath_hw *ah,
static bool ath9k_hw_channel_change(struct ath_hw *ah,
struct ath9k_channel *chan)
{
- struct ath_regulatory *regulatory = ath9k_hw_regulatory(ah);
struct ath_common *common = ath9k_hw_common(ah);
- struct ieee80211_channel *channel = chan->chan;
u32 qnum;
int r;
@@ -1411,14 +1408,7 @@ static bool ath9k_hw_channel_change(struct ath_hw *ah,
return false;
}
ath9k_hw_set_clockrate(ah);
-
- ah->eep_ops->set_txpower(ah, chan,
- ath9k_regd_get_ctl(regulatory, chan),
- channel->max_antenna_gain * 2,
- channel->max_power * 2,
- min((u32) MAX_RATE_POWER,
- (u32) regulatory->power_limit), false);
-
+ ath9k_hw_apply_txpower(ah, chan);
ath9k_hw_rfbus_done(ah);
if (IS_CHAN_OFDM(chan) || IS_CHAN_HT(chan))
@@ -2489,23 +2479,56 @@ bool ath9k_hw_disable(struct ath_hw *ah)
}
EXPORT_SYMBOL(ath9k_hw_disable);
+static int get_antenna_gain(struct ath_hw *ah, struct ath9k_channel *chan)
+{
+ enum eeprom_param gain_param;
+
+ if (IS_CHAN_2GHZ(chan))
+ gain_param = EEP_ANTENNA_GAIN_2G;
+ else
+ gain_param = EEP_ANTENNA_GAIN_5G;
+
+ return ah->eep_ops->get_eeprom(ah, gain_param);
+}
+
+void ath9k_hw_apply_txpower(struct ath_hw *ah, struct ath9k_channel *chan)
+{
+ struct ath_regulatory *reg = ath9k_hw_regulatory(ah);
+ struct ieee80211_channel *channel;
+ int chan_pwr, new_pwr, max_gain;
+ int ant_gain, ant_reduction = 0;
+
+ if (!chan)
+ return;
+
+ channel = chan->chan;
+ chan_pwr = min_t(int, channel->max_power * 2, MAX_RATE_POWER);
+ new_pwr = min_t(int, chan_pwr, reg->power_limit);
+ max_gain = new_pwr - chan_pwr + channel->max_antenna_gain * 2;
+
+ ant_gain = get_antenna_gain(ah, chan);
+ if (ant_gain > max_gain)
+ ant_reduction = ant_gain - max_gain;
+
+ ah->eep_ops->set_txpower(ah, chan,
+ ath9k_regd_get_ctl(reg, chan),
+ ant_reduction, new_pwr, false);
+}
+
void ath9k_hw_set_txpowerlimit(struct ath_hw *ah, u32 limit, bool test)
{
- struct ath_regulatory *regulatory = ath9k_hw_regulatory(ah);
+ struct ath_regulatory *reg = ath9k_hw_regulatory(ah);
struct ath9k_channel *chan = ah->curchan;
struct ieee80211_channel *channel = chan->chan;
- int reg_pwr = min_t(int, MAX_RATE_POWER, limit);
- int chan_pwr = channel->max_power * 2;
+ reg->power_limit = min_t(int, limit * 2, MAX_RATE_POWER);
if (test)
- reg_pwr = chan_pwr = MAX_RATE_POWER;
+ channel->max_power = MAX_RATE_POWER / 2;
- regulatory->power_limit = reg_pwr;
+ ath9k_hw_apply_txpower(ah, chan);
- ah->eep_ops->set_txpower(ah, chan,
- ath9k_regd_get_ctl(regulatory, chan),
- channel->max_antenna_gain * 2,
- chan_pwr, reg_pwr, test);
+ if (test)
+ channel->max_power = DIV_ROUND_UP(reg->max_power_level, 2);
}
EXPORT_SYMBOL(ath9k_hw_set_txpowerlimit);
diff --git a/drivers/net/wireless/ath/ath9k/hw.h b/drivers/net/wireless/ath/ath9k/hw.h
index 24889f7..684c33c 100644
--- a/drivers/net/wireless/ath/ath9k/hw.h
+++ b/drivers/net/wireless/ath/ath9k/hw.h
@@ -390,14 +390,6 @@ enum ath9k_power_mode {
ATH9K_PM_UNDEFINED
};
-enum ath9k_tp_scale {
- ATH9K_TP_SCALE_MAX = 0,
- ATH9K_TP_SCALE_50,
- ATH9K_TP_SCALE_25,
- ATH9K_TP_SCALE_12,
- ATH9K_TP_SCALE_MIN
-};
-
enum ser_reg_mode {
SER_REG_MODE_OFF = 0,
SER_REG_MODE_ON = 1,
@@ -968,6 +960,7 @@ void ath9k_hw_htc_resetinit(struct ath_hw *ah);
/* PHY */
void ath9k_hw_get_delta_slope_vals(struct ath_hw *ah, u32 coef_scaled,
u32 *coef_mantissa, u32 *coef_exponent);
+void ath9k_hw_apply_txpower(struct ath_hw *ah, struct ath9k_channel *chan);
/*
* Code Specific to AR5008, AR9001 or AR9002,
diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
index 39514de..af1b325 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -626,7 +626,6 @@ static void ath9k_init_band_txpower(struct ath_softc *sc, int band)
struct ieee80211_supported_band *sband;
struct ieee80211_channel *chan;
struct ath_hw *ah = sc->sc_ah;
- struct ath_regulatory *reg = ath9k_hw_regulatory(ah);
int i;
sband = &sc->sbands[band];
@@ -635,7 +634,6 @@ static void ath9k_init_band_txpower(struct ath_softc *sc, int band)
ah->curchan = &ah->channels[chan->hw_value];
ath9k_cmn_update_ichannel(ah->curchan, chan, NL80211_CHAN_HT20);
ath9k_hw_set_txpowerlimit(ah, MAX_RATE_POWER, true);
- chan->max_power = reg->max_power_level / 2;
}
}
--
1.7.3.2
On 2011-09-19 11:45 PM, Luis R. Rodriguez wrote:
> On Mon, Sep 19, 2011 at 2:21 PM, Felix Fietkau<[email protected]> wrote:
>> On 2011-09-19 11:14 PM, Luis R. Rodriguez wrote:
>>>
>>> On Mon, Sep 19, 2011 at 1:50 PM, Felix Fietkau<[email protected]> wrote:
>>>>
>>>> On 2011-09-19 10:41 PM, Luis R. Rodriguez wrote:
>>>>>
>>>>> On Mon, Sep 19, 2011 at 10:38 AM, Felix Fietkau<[email protected]>
>>>>> wrote:
>>>>>>
>>>>>> @@ -986,21 +995,15 @@ static void
>>>>>> ath9k_hw_set_def_power_per_rate_table(struct ath_hw *ah,
>>>>>> struct ath9k_channel
>>>>>> *chan,
>>>>>> int16_t *ratesArray,
>>>>>> u16 cfgCtl,
>>>>>> - u16
>>>>>> AntennaReduction,
>>>>>> - u16
>>>>>> twiceMaxRegulatoryPower,
>>>>>> + u16
>>>>>> antenna_reduction,
>>>>>> u16 powerLimit)
>>>>>> {
>>>>>> #define REDUCE_SCALED_POWER_BY_TWO_CHAIN 6 /* 10*log10(2)*2 */
>>>>>> #define REDUCE_SCALED_POWER_BY_THREE_CHAIN 9 /* 10*log10(3)*2 */
>>>>>>
>>>>>> - struct ath_regulatory *regulatory = ath9k_hw_regulatory(ah);
>>>>>> struct ar5416_eeprom_def *pEepData =&ah->eeprom.def;
>>>>>> u16 twiceMaxEdgePower = MAX_RATE_POWER;
>>>>>> - static const u16 tpScaleReductionTable[5] =
>>>>>> - { 0, 3, 6, 9, MAX_RATE_POWER };
>>>>>> -
>>>>>> int i;
>>>>>> - int16_t twiceLargestAntenna;
>>>>>> struct cal_ctl_data *rep;
>>>>>> struct cal_target_power_leg targetPowerOfdm, targetPowerCck =
>>>>>> {
>>>>>> 0, { 0, 0, 0, 0}
>>>>>> @@ -1012,7 +1015,7 @@ static void
>>>>>> ath9k_hw_set_def_power_per_rate_table(struct ath_hw *ah,
>>>>>> struct cal_target_power_ht targetPowerHt20, targetPowerHt40 =
>>>>>> {
>>>>>> 0, {0, 0, 0, 0}
>>>>>> };
>>>>>> - u16 scaledPower = 0, minCtlPower, maxRegAllowedPower;
>>>>>> + u16 scaledPower = 0, minCtlPower;
>>>>>> static const u16 ctlModesFor11a[] = {
>>>>>> CTL_11A, CTL_5GHT20, CTL_11A_EXT, CTL_5GHT40
>>>>>> };
>>>>>
>>>>> Although we do not use the reg->tp_scale I see no log which explains
>>>>> that it will not, I'm reviewing TPC right now and we do need to
>>>>> support it but its unclear to me yet if we are doing everything
>>>>> correctly in mac80211 / cfg80211 for it. As far as I can see the
>>>>> tp_scale stuff is used only if the default is not set, but as you
>>>>> noted its always set to the default. I am trying to review the
>>>>> internal code to see under what cases this changes but its hard to
>>>>> review. Although I'd like to see this removed I'd prefer to treat this
>>>>> separately from the cleanup you are doing, which I do find highly
>>>>> valuable.
>>>>
>>>> Why keep it? All it does is subtract something from the max regulatory
>>>> power
>>>> level, so it does not make any sense to keep this crap duplicated in all
>>>> the
>>>> the eeprom files.
>>>
>>> You're right that they do seem to use the same array values, but the
>>> fact that its all common code is separate from the question of
>>> removing it.
>>
>> Right now it's dead code. If we need it later, we should just rewrite it. I
>> think carring forward old dead code just in case we might use it later is
>> not a good idea.
>
> I agree with if the code provides no value, but I do not feel you have
> proven this.
It simply provides some knobs to set the power to 50%, 25%, 12.5% or
minimum. Why do I have to prove the lack of value of completely dead
code, which is unused even in the codebase that it came from?
>>>> If we ever do need it (which I doubt),
>>>
>>> I suspect this is needed for APs that support DFS and since we do not
>>> yet support DFS I do not think this is used. I am not 100% certain yet
>>> but at least in my review in the last few minutes it seems the AP can
>>> decide to change TPC constraints dynamically based on TPC reports from
>>> the STAs. I suspec this is where this comes in to play.
>>
>> I looked at the other ath driver and I see no indication that it's related
>> to DFS in any way.
>
> I have verified this just now as well, it seems it was only used to
> support an ioctl to userspace to enable users to update a tpscale
> value but I see no documentation about this. Next question is who in
> usersapce sets this. I wonder if its done through userspace after
> measuring some TPC reports from STAs.
Even in the default userspace of the ath stuff nothing actually uses this.
>> Even if it is, it doesn't even belong in ath9k. Any form
>> of tx power reduction can be handled by cfg80211/mac80211.
>
> Depends, general hooks can surely go into cfg80211 / mac80211 but if
> any decision is being made to adjust power due to some environmental
> constraints this would likely require chipset specific knobs or
> information.
I really don't see how it would need chip specific hooks.
>>>> it needs to be added in a different place anyway.
>>>
>>> If its card specific so I do not think we can move any of this
>>> commonly into cfg80211 / mac80211.
>>
>> It's not card specific in any way.
>
> In code right now its all the same arrays, but does it mean it will
> not differ from broadcom's cards?
Heh, do you really think that dB means something else for Broadcom than
it does for Atheros? :)
>>> One thing is to simply common code, another is to remove code which we
>>> is not enabled right now, but may be later. I think we should treat
>>> these separately.
>>
>> I don't think we'll ever enable this, and it's not significant enough to
>> keep it around.
>
> I am not yet convinced its insignificant. I'd like to better
> understand first who sets this and why before removing it. If anything
> at least a separate patch should deal with the removal.
This is bitrot that has been carried forward since the old AR5212 days.
It has been forward ported because nobody bothered to remove it. It's
even present in old madwifi releases.
- Felix
On 2011-09-19 11:54 PM, Luis R. Rodriguez wrote:
> On Mon, Sep 19, 2011 at 2:45 PM, Luis R. Rodriguez
> <[email protected]> wrote:
>> On Mon, Sep 19, 2011 at 2:21 PM, Felix Fietkau<[email protected]> wrote:
>>> I looked at the other ath driver and I see no indication that it's related
>>> to DFS in any way.
>>
>> I have verified this just now as well, it seems it was only used to
>> support an ioctl to userspace to enable users to update a tpscale
>> value but I see no documentation about this. Next question is who in
>> usersapce sets this. I wonder if its done through userspace after
>> measuring some TPC reports from STAs.
>
> So this comes from supporting a "TR-098" specification, which seems to
> be the "Internet Gateway Device data model for the CPE WAN Management
> Protocol". I haven't yet been able to map this to the specification
> respective component:
>
> http://www.broadband-forum.org/technical/download/TR-098.pdf
Interesting. That definitely supports my point that ath9k is the wrong
place for something like this to be. Let's just get rid of it.
- Felix
IIRC, the TP Scale stuff dates back to the previous NICs which
(reportedly, from madwifi list/tickets) didn't implement TPC right, or
Madwifi never ran TPC correctly on them.
So instead of per-packet TPC being done as driven by the net80211
stack (which I bet would be needed for hostap mode, to use per-node TX
power levels), it was likely done as a way for STA's to select a power
limit, and maybe for coarse grained hostap operation (ie, if all STA's
are close, use a lower TPC scale.)
I bet Sam would know why.
I agree with Felix; nothing in ath9k/mac80211 currently uses it, so
why keep it around. All of the ath9k supported NICs should implement
correct per-packet TPC anyway, right?
Adrian