Search Linux Wireless

Re: [PATCH v2 2/4] ath9k_hw: clean up tx power handling

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2011-09-19 11:14 PM, Luis R. Rodriguez wrote:
On Mon, Sep 19, 2011 at 1:50 PM, Felix Fietkau<nbd@xxxxxxxxxxx>  wrote:
 On 2011-09-19 10:41 PM, Luis R. Rodriguez wrote:

 On Mon, Sep 19, 2011 at 10:38 AM, Felix Fietkau<nbd@xxxxxxxxxxx>    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
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Host AP]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Linux Kernel]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]
  Powered by Linux