[rt2x00-users] [PATCH 1/3] rt2x00: Fix rt2800 txpower setting to correct value
Helmut Schaa
helmut.schaa at googlemail.com
Mon Jan 17 19:48:57 EST 2011
Hi Jay,
See some comments below.
Am Montag, 17. Januar 2011 schrieb Jay_Hung at ralinktech.com:
> From: RA-Jay Hung <jay_hung at ralinktech.com>
>
> TX_PWR_CFG_* setting need to consider below cases
> -20M/40M power delta for 2.4/5GHZ band
> -limit maximum EIRP tx power to power_level of
> regulatory requirement
> -ALC tuning as temperature change(TODO)
What does the abbreviation ALC stand for?
As there is still a TODO on the list this series is meant as RFC?
> Signed-off-by: RA-Jay Hung <jay_hung at ralinktech.com>
> ---
> drivers/net/wireless/rt2x00/rt2800.h | 22 +-
> drivers/net/wireless/rt2x00/rt2800lib.c | 366 +++++++++++++++++++---------
> drivers/net/wireless/rt2x00/rt2x00.h | 34 +++
> drivers/net/wireless/rt2x00/rt2x00config.c | 19 ++-
> 4 files changed, 309 insertions(+), 132 deletions(-)
>
> diff --git a/drivers/net/wireless/rt2x00/rt2800.h b/drivers/net/wireless/rt2x00/rt2800.h
> index c7e615c..ed7bdd0 100644
> --- a/drivers/net/wireless/rt2x00/rt2800.h
> +++ b/drivers/net/wireless/rt2x00/rt2800.h
> @@ -1697,11 +1697,11 @@ struct mac_iveiv_entry {
> */
>
> /*
> - * BBP 1: TX Antenna & Power
> - * POWER: 0 - normal, 1 - drop tx power by 6dBm, 2 - drop tx power by 12dBm,
> + * BBP 1: TX Antenna & Power Control
> + * POWER_CTRL: 0 - normal, 1 - drop tx power by 6dBm, 2 - drop tx power by 12dBm,
This line is >80 chars.
> * 3 - increase tx power by 6dBm
> */
> -#define BBP1_TX_POWER FIELD8(0x07)
> +#define BBP1_TX_POWER_CTRL FIELD8(0x07)
> #define BBP1_TX_ANTENNA FIELD8(0x18)
>
> /*
> @@ -1995,23 +1995,23 @@ struct mac_iveiv_entry {
> #define EEPROM_RSSI_A2_LNA_A2 FIELD16(0xff00)
>
> /*
> - * EEPROM Maximum TX power values
> + * EEPROM EIRP Maximum TX power values
Would it be possible to add a comment here about which unit the eeprom
values are in? dBm? But of course this can live in a second patch but
it would help a lot to better understand the tx power handling. The same
of course applies to all txpower related eeprom values.
Thanks.
> */
> -#define EEPROM_MAX_TX_POWER 0x0027
> -#define EEPROM_MAX_TX_POWER_24GHZ FIELD16(0x00ff)
> -#define EEPROM_MAX_TX_POWER_5GHZ FIELD16(0xff00)
> +#define EEPROM_EIRP_MAX_TX_POWER 0x0027
> +#define EEPROM_EIRP_MAX_TX_POWER_2GHZ FIELD16(0x00ff)
> +#define EEPROM_EIRP_MAX_TX_POWER_5GHZ FIELD16(0xff00)
>
> /*
> * EEPROM TXpower delta: 20MHZ AND 40 MHZ use different power.
> * This is delta in 40MHZ.
> * VALUE: Tx Power dalta value (MAX=4)
> * TYPE: 1: Plus the delta value, 0: minus the delta value
> - * TXPOWER: Enable:
> + * ENABLE: enable tx power compensation for 40BW
> */
> #define EEPROM_TXPOWER_DELTA 0x0028
> -#define EEPROM_TXPOWER_DELTA_VALUE FIELD16(0x003f)
> -#define EEPROM_TXPOWER_DELTA_TYPE FIELD16(0x0040)
> -#define EEPROM_TXPOWER_DELTA_TXPOWER FIELD16(0x0080)
> +#define EEPROM_TXPOWER_DELTA_VALUE FIELD8(0x3f)
> +#define EEPROM_TXPOWER_DELTA_TYPE FIELD8(0x40)
> +#define EEPROM_TXPOWER_DELTA_ENABLE FIELD8(0x80)
Is there a specific reason for reducing the width to 8 bit?
> /*
> * EEPROM TXPOWER 802.11BG
> diff --git a/drivers/net/wireless/rt2x00/rt2800lib.c b/drivers/net/wireless/rt2x00/rt2800lib.c
> index 3fc1e57..d4a4473 100644
> --- a/drivers/net/wireless/rt2x00/rt2800lib.c
> +++ b/drivers/net/wireless/rt2x00/rt2800lib.c
> @@ -978,6 +978,188 @@ static void rt2800_init_led(struct rt2x00_dev *rt2x00dev,
> }
> #endif /* CONFIG_RT2X00_LIB_LEDS */
>
> +static void rt2800_txpower_delta(struct rt2x00_dev *rt2x00dev,
> + u8 txpower_delta, u32 *txpower_cfg_20M, u32 *txpower_cfg_40M)
Line >80 chars.
> +{
> + int i;
> + u8 comp_en;
> + u8 comp_type;
> + char comp_value;
> +
> + comp_en = rt2x00_get_field8(txpower_delta,
> + EEPROM_TXPOWER_DELTA_ENABLE);
> + if (comp_en) {
> + comp_type = rt2x00_get_field8(txpower_delta,
> + EEPROM_TXPOWER_DELTA_TYPE);
> + comp_value = rt2x00_get_field8(txpower_delta,
> + EEPROM_TXPOWER_DELTA_VALUE);
> +
Ah comment like "Use the delta as positive or negative value based on the
eeprom settings" would help to make the code better understandable here.
> + if (!comp_type)
> + comp_value = -comp_value;
> +
> + for (i = 0; i < TXPOWER_CFG_NUM; i++)
> + txpower_cfg_40M[i] = txpower_cfg_20M[i] + comp_value;
Line >80 chars.
> + }
> +}
> +
> +static void rt2800_init_txpower(struct rt2x00_dev *rt2x00dev,
> + struct rt2x00_txpower_info *info)
> +{
> + u8 txpower;
> + u16 eeprom;
> + u32 offset;
> + u32 reg;
> + int i, j = 0;
> + u8 txpower_delta;
> + u8 r1;
> +
> + /*
> + * set to normal bbp tx power control mode: +/- 0dBm
> + */
> + rt2800_bbp_read(rt2x00dev, 1, &r1);
> + rt2x00_set_field8(&r1, BBP1_TX_POWER_CTRL, 0);
> + rt2800_bbp_write(rt2x00dev, 1, r1);
> + offset = TX_PWR_CFG_0;
> +
> + for (i = 0; i < EEPROM_TXPOWER_BYRATE_SIZE; i += 2) {
> + /* just to be safe */
> + if (offset > TX_PWR_CFG_4)
> + break;
> +
> + rt2800_register_read(rt2x00dev, offset, ®);
> +
> + /* read the next four txpower values */
> + rt2x00_eeprom_read(rt2x00dev, EEPROM_TXPOWER_BYRATE + i,
> + &eeprom);
> +
> + /*
> + * TX_PWR_CFG_0: 1MBS, TX_PWR_CFG_1: 24MBS,
> + * TX_PWR_CFG_2: MCS4, TX_PWR_CFG_3: MCS12,
> + * TX_PWR_CFG_4: unknown
> + */
> + txpower = rt2x00_get_field16(eeprom,
> + EEPROM_TXPOWER_BYRATE_RATE0);
> + rt2x00_set_field32(®, TX_PWR_CFG_RATE0, txpower);
> +
> + /*
> + * TX_PWR_CFG_0: 2MBS, TX_PWR_CFG_1: 36MBS,
> + * TX_PWR_CFG_2: MCS5, TX_PWR_CFG_3: MCS13,
> + * TX_PWR_CFG_4: unknown
> + */
> + txpower = rt2x00_get_field16(eeprom,
> + EEPROM_TXPOWER_BYRATE_RATE1);
> + rt2x00_set_field32(®, TX_PWR_CFG_RATE1, txpower);
> +
> + /*
> + * TX_PWR_CFG_0: 55MBS, TX_PWR_CFG_1: 48MBS,
> + * TX_PWR_CFG_2: MCS6, TX_PWR_CFG_3: MCS14,
> + * TX_PWR_CFG_4: unknown
> + */
> + txpower = rt2x00_get_field16(eeprom,
> + EEPROM_TXPOWER_BYRATE_RATE2);
> + rt2x00_set_field32(®, TX_PWR_CFG_RATE2, txpower);
> +
> + /*
> + * TX_PWR_CFG_0: 11MBS, TX_PWR_CFG_1: 54MBS,
> + * TX_PWR_CFG_2: MCS7, TX_PWR_CFG_3: MCS15,
> + * TX_PWR_CFG_4: unknown
> + */
> + txpower = rt2x00_get_field16(eeprom,
> + EEPROM_TXPOWER_BYRATE_RATE3);
> + rt2x00_set_field32(®, TX_PWR_CFG_RATE3, txpower);
> +
> + /* read the next four txpower values */
> + rt2x00_eeprom_read(rt2x00dev, EEPROM_TXPOWER_BYRATE + i + 1,
> + &eeprom);
> +
> + /*
> + * TX_PWR_CFG_0: 6MBS, TX_PWR_CFG_1: MCS0,
> + * TX_PWR_CFG_2: MCS8, TX_PWR_CFG_3: unknown,
> + * TX_PWR_CFG_4: unknown
> + */
> + txpower = rt2x00_get_field16(eeprom,
> + EEPROM_TXPOWER_BYRATE_RATE0);
> + rt2x00_set_field32(®, TX_PWR_CFG_RATE4, txpower);
> +
> + /*
> + * TX_PWR_CFG_0: 9MBS, TX_PWR_CFG_1: MCS1,
> + * TX_PWR_CFG_2: MCS9, TX_PWR_CFG_3: unknown,
> + * TX_PWR_CFG_4: unknown
> + */
> + txpower = rt2x00_get_field16(eeprom,
> + EEPROM_TXPOWER_BYRATE_RATE1);
> + rt2x00_set_field32(®, TX_PWR_CFG_RATE5, txpower);
> +
> + /*
> + * TX_PWR_CFG_0: 12MBS, TX_PWR_CFG_1: MCS2,
> + * TX_PWR_CFG_2: MCS10, TX_PWR_CFG_3: unknown,
> + * TX_PWR_CFG_4: unknown
> + */
> + txpower = rt2x00_get_field16(eeprom,
> + EEPROM_TXPOWER_BYRATE_RATE2);
> + rt2x00_set_field32(®, TX_PWR_CFG_RATE6, txpower);
> +
> + /*
> + * TX_PWR_CFG_0: 18MBS, TX_PWR_CFG_1: MCS3,
> + * TX_PWR_CFG_2: MCS11, TX_PWR_CFG_3: unknown,
> + * TX_PWR_CFG_4: unknown
> + */
> + txpower = rt2x00_get_field16(eeprom,
> + EEPROM_TXPOWER_BYRATE_RATE3);
> + rt2x00_set_field32(®, TX_PWR_CFG_RATE7, txpower);
> +
> + rt2800_register_write(rt2x00dev, offset, reg);
> +
> + info->txpower_20M_2GHZ[j] = reg;
> + info->txpower_20M_5GHZ[j] = reg;
> + j++;
> +
> + /* next TX_PWR_CFG register */
> + offset += 4;
> + }
> +
> + /*
> + * Get power delta under 40M BW for 2.4G/5G band
> + */
> + rt2x00_eeprom_read(rt2x00dev, EEPROM_TXPOWER_DELTA, &eeprom);
> +
> + for (i = 0; i < IEEE80211_NUM_BANDS; i++) {
> + txpower_delta = (eeprom >> (i * 8)) & 0x00FF;
> + if (i == IEEE80211_BAND_2GHZ) {
> + if (txpower_delta != 0xff)
> + rt2800_txpower_delta(rt2x00dev, txpower_delta,
> + info->txpower_20M_2GHZ, info->txpower_40M_2GHZ);
> + else {
> + for (j = 0; j < TXPOWER_CFG_NUM; j++)
> + info->txpower_40M_2GHZ[j] = info->txpower_20M_2GHZ[j];
> + }
> + } else if(i == IEEE80211_BAND_5GHZ) {
> + if (txpower_delta != 0xff)
> + rt2800_txpower_delta(rt2x00dev, txpower_delta,
> + info->txpower_20M_5GHZ, info->txpower_40M_5GHZ);
> + else {
> + for (j = 0; j < TXPOWER_CFG_NUM; j++)
> + info->txpower_40M_5GHZ[j] = info->txpower_20M_5GHZ[j];
> + }
> + }
> + }
> +
> + info->default_txpower = info->txpower_20M_2GHZ;
> + info->txpower_limit = 35;
> +
> + /*
> + * Get EIRP tx power for 2.4G/5G band
> + */
> + rt2x00_eeprom_read(rt2x00dev, EEPROM_EIRP_MAX_TX_POWER, &eeprom);
> + info->eirp_txpower[IEEE80211_BAND_2GHZ] =
> + rt2x00_get_field16(eeprom, EEPROM_EIRP_MAX_TX_POWER_2GHZ);
> + info->eirp_txpower[IEEE80211_BAND_5GHZ] =
> + rt2x00_get_field16(eeprom, EEPROM_EIRP_MAX_TX_POWER_5GHZ);
> +
> + if (info->eirp_txpower[IEEE80211_BAND_2GHZ] < 0x50)
> + __set_bit(CONFIG_SUPPORT_POWER_LIMIT, &rt2x00dev->flags);
> +}
> +
This adds quite some redundant code. I'd say it makes sense to use
rt2800_config_txpower for tx power initialization instead of open coding
it here again.
Second, is it really necessary to keep a tx power array in memory? I mean,
the eeprom data is already in memory, so these reads are not expensive.
And as far as I understood your patch we configure the tx power values
based on the tx power values stored in the eeprom and apply different
compensation values to them (band-based, temperature-based and 40Mhz-based).
So, we could simply reuse the code above and calculate the txpower values
on the fly based on the actual copnfiguration (band, 40Mhz etc.).
The tx power changes are not in an actual hotpath, and since we have to
recalculate the txpower array during channel change anyway it wouldn't hurt
to do it all on-the-fly.
I thought about something like this:
rt2800_config_txpower(int band_comp, int bw_comp, ...
[...]
for (i = 0; i < EEPROM_TXPOWER_BYRATE_SIZE; i += 2) {
[...]
/*
* TX_PWR_CFG_0: 1MBS, TX_PWR_CFG_1: 24MBS,
* TX_PWR_CFG_2: MCS4, TX_PWR_CFG_3: MCS12,
* TX_PWR_CFG_4: unknown
*/
txpower = rt2x00_get_field16(eeprom,
EEPROM_TXPOWER_BYRATE_RATE0);
/*
* Add band, 40Mhz and temperature compensation
*/
txpower += band_comp + bw_comp + temp_comp;
rt2x00_set_field32(®, TX_PWR_CFG_RATE0, txpower);
[...]
This function could then be used for the register initialization but also
for tx power configuration (triggerd by regulatory change or by channel
change).
> /*
> * Configuration handlers.
> */
> @@ -1645,113 +1827,65 @@ static void rt2800_config_channel(struct rt2x00_dev *rt2x00dev,
> }
>
> static void rt2800_config_txpower(struct rt2x00_dev *rt2x00dev,
> - const int max_txpower)
> -{
> - u8 txpower;
> - u8 max_value = (u8)max_txpower;
> - u16 eeprom;
> - int i;
> - u32 reg;
> - u8 r1;
> - u32 offset;
> -
> - /*
> - * set to normal tx power mode: +/- 0dBm
> - */
> - rt2800_bbp_read(rt2x00dev, 1, &r1);
> - rt2x00_set_field8(&r1, BBP1_TX_POWER, 0);
> - rt2800_bbp_write(rt2x00dev, 1, r1);
> + struct rt2x00_txpower_info *info, unsigned long flags)
Why pass flags as individual argument, it is already part of rt2x00_txpower_info, no?
> +{
> + u32 offset = TX_PWR_CFG_0;
> + int i, j;
> + u8 criterion;
> + char total_delta, reg_delta;
> + u32 max_txpower_per_rate;
> + u8 txpower_per_rate;
> + u32 *default_txpower = info->default_txpower;
> + u32 adjusted_txpower[TXPOWER_CFG_NUM];
> + enum ieee80211_band curr_band = rt2x00dev->curr_band;
> +
> + if (test_and_clear_bit(TXPOWER_CHANNEL_CONF_CHANGE, &flags)) {
> + if (curr_band == IEEE80211_BAND_2GHZ) {
> + if(test_bit(CONFIG_CHANNEL_HT40, &rt2x00dev->flags))
> + default_txpower = info->txpower_40M_2GHZ;
> + else
> + default_txpower = info->txpower_20M_2GHZ;
> + } else if (curr_band == IEEE80211_BAND_5GHZ) {
> + if(test_bit(CONFIG_CHANNEL_HT40, &rt2x00dev->flags))
> + default_txpower = info->txpower_40M_5GHZ;
> + else
> + default_txpower = info->txpower_20M_5GHZ;
> + }
> + }
>
> /*
> - * The eeprom contains the tx power values for each rate. These
> - * values map to 100% tx power. Each 16bit word contains four tx
> - * power values and the order is the same as used in the TX_PWR_CFG
> - * registers.
> + * Check if exceed txpower_limit
> + * we use OFDM 6M as criterion
> + * CCK need add additional 4dbm.
> */
> - offset = TX_PWR_CFG_0;
> -
> - for (i = 0; i < EEPROM_TXPOWER_BYRATE_SIZE; i += 2) {
> - /* just to be safe */
> - if (offset > TX_PWR_CFG_4)
> - break;
> -
> - rt2800_register_read(rt2x00dev, offset, ®);
> -
> - /* read the next four txpower values */
> - rt2x00_eeprom_read(rt2x00dev, EEPROM_TXPOWER_BYRATE + i,
> - &eeprom);
> -
> - /* TX_PWR_CFG_0: 1MBS, TX_PWR_CFG_1: 24MBS,
> - * TX_PWR_CFG_2: MCS4, TX_PWR_CFG_3: MCS12,
> - * TX_PWR_CFG_4: unknown */
> - txpower = rt2x00_get_field16(eeprom,
> - EEPROM_TXPOWER_BYRATE_RATE0);
> - rt2x00_set_field32(®, TX_PWR_CFG_RATE0,
> - min(txpower, max_value));
> -
> - /* TX_PWR_CFG_0: 2MBS, TX_PWR_CFG_1: 36MBS,
> - * TX_PWR_CFG_2: MCS5, TX_PWR_CFG_3: MCS13,
> - * TX_PWR_CFG_4: unknown */
> - txpower = rt2x00_get_field16(eeprom,
> - EEPROM_TXPOWER_BYRATE_RATE1);
> - rt2x00_set_field32(®, TX_PWR_CFG_RATE1,
> - min(txpower, max_value));
> -
> - /* TX_PWR_CFG_0: 55MBS, TX_PWR_CFG_1: 48MBS,
> - * TX_PWR_CFG_2: MCS6, TX_PWR_CFG_3: MCS14,
> - * TX_PWR_CFG_4: unknown */
> - txpower = rt2x00_get_field16(eeprom,
> - EEPROM_TXPOWER_BYRATE_RATE2);
> - rt2x00_set_field32(®, TX_PWR_CFG_RATE2,
> - min(txpower, max_value));
> -
> - /* TX_PWR_CFG_0: 11MBS, TX_PWR_CFG_1: 54MBS,
> - * TX_PWR_CFG_2: MCS7, TX_PWR_CFG_3: MCS15,
> - * TX_PWR_CFG_4: unknown */
> - txpower = rt2x00_get_field16(eeprom,
> - EEPROM_TXPOWER_BYRATE_RATE3);
> - rt2x00_set_field32(®, TX_PWR_CFG_RATE3,
> - min(txpower, max_value));
> -
> - /* read the next four txpower values */
> - rt2x00_eeprom_read(rt2x00dev, EEPROM_TXPOWER_BYRATE + i + 1,
> - &eeprom);
> + criterion = rt2x00_get_field32(default_txpower[0], TX_PWR_CFG_0_6MBS);
>
> - /* TX_PWR_CFG_0: 6MBS, TX_PWR_CFG_1: MCS0,
> - * TX_PWR_CFG_2: MCS8, TX_PWR_CFG_3: unknown,
> - * TX_PWR_CFG_4: unknown */
> - txpower = rt2x00_get_field16(eeprom,
> - EEPROM_TXPOWER_BYRATE_RATE0);
> - rt2x00_set_field32(®, TX_PWR_CFG_RATE4,
> - min(txpower, max_value));
> + for (i = 0; i < TXPOWER_CFG_NUM; i++) {
> + for (j = 0; j < 8; j++) {
> + txpower_per_rate = (default_txpower[i] >> (j * 4)) & 0x0f;
>
> - /* TX_PWR_CFG_0: 9MBS, TX_PWR_CFG_1: MCS1,
> - * TX_PWR_CFG_2: MCS9, TX_PWR_CFG_3: unknown,
> - * TX_PWR_CFG_4: unknown */
> - txpower = rt2x00_get_field16(eeprom,
> - EEPROM_TXPOWER_BYRATE_RATE1);
> - rt2x00_set_field32(®, TX_PWR_CFG_RATE5,
> - min(txpower, max_value));
> + if (test_bit(CONFIG_SUPPORT_POWER_LIMIT, &rt2x00dev->flags)) {
> + if ((i == 0) && (j < 4))
> + max_txpower_per_rate = info->eirp_txpower[curr_band]
> + + txpower_per_rate - criterion + 4;
> + else
> + max_txpower_per_rate = info->eirp_txpower[curr_band]
> + + txpower_per_rate - criterion;
>
> - /* TX_PWR_CFG_0: 12MBS, TX_PWR_CFG_1: MCS2,
> - * TX_PWR_CFG_2: MCS10, TX_PWR_CFG_3: unknown,
> - * TX_PWR_CFG_4: unknown */
> - txpower = rt2x00_get_field16(eeprom,
> - EEPROM_TXPOWER_BYRATE_RATE2);
> - rt2x00_set_field32(®, TX_PWR_CFG_RATE6,
> - min(txpower, max_value));
> + if (max_txpower_per_rate > info->txpower_limit)
> + reg_delta = max_txpower_per_rate - info->txpower_limit;
> + else
> + reg_delta = 0;
> + } else
> + reg_delta = 0;
>
> - /* TX_PWR_CFG_0: 18MBS, TX_PWR_CFG_1: MCS3,
> - * TX_PWR_CFG_2: MCS11, TX_PWR_CFG_3: unknown,
> - * TX_PWR_CFG_4: unknown */
> - txpower = rt2x00_get_field16(eeprom,
> - EEPROM_TXPOWER_BYRATE_RATE3);
> - rt2x00_set_field32(®, TX_PWR_CFG_RATE7,
> - min(txpower, max_value));
> + total_delta = reg_delta;
>
> - rt2800_register_write(rt2x00dev, offset, reg);
> + adjusted_txpower[i] = default_txpower[i] & ~(0x0f << (j * 4));
> + adjusted_txpower[i] |= ((txpower_per_rate - total_delta) & 0x0f) << (j * 4);
> + }
>
> - /* next TX_PWR_CFG register */
> + rt2800_register_write(rt2x00dev, offset, adjusted_txpower[i]);
> offset += 4;
> }
> }
> @@ -1806,11 +1940,17 @@ void rt2800_config(struct rt2x00_dev *rt2x00dev,
> /* Always recalculate LNA gain before changing configuration */
> rt2800_config_lna_gain(rt2x00dev, libconf);
>
> - if (flags & IEEE80211_CONF_CHANGE_CHANNEL)
> + if (flags & IEEE80211_CONF_CHANGE_CHANNEL) {
> rt2800_config_channel(rt2x00dev, libconf->conf,
> &libconf->rf, &libconf->channel);
> - if (flags & IEEE80211_CONF_CHANGE_POWER)
> - rt2800_config_txpower(rt2x00dev, libconf->conf->power_level);
> + rt2800_config_txpower(rt2x00dev, &rt2x00dev->txpower_info,
> + rt2x00dev->txpower_info.flags);
> + }
> + if (flags & IEEE80211_CONF_CHANGE_POWER) {
> + rt2x00dev->txpower_info.txpower_limit = libconf->conf->power_level;
> + rt2800_config_txpower(rt2x00dev, &rt2x00dev->txpower_info,
> + rt2x00dev->txpower_info.flags);
> + }
> if (flags & IEEE80211_CONF_CHANGE_RETRY_LIMITS)
> rt2800_config_retry_limit(rt2x00dev, libconf);
> if (flags & IEEE80211_CONF_CHANGE_PS)
> @@ -2017,6 +2157,9 @@ static int rt2800_init_registers(struct rt2x00_dev *rt2x00dev)
>
> rt2800_register_write(rt2x00dev, PBF_MAX_PCNT, 0x1f3fbf9f);
>
> + /* Initialize TX_PWR_CFG_* base on 20M bandwidth */
> + rt2800_init_txpower(rt2x00dev, &rt2x00dev->txpower_info);
> +
> rt2800_register_read(rt2x00dev, TX_RTY_CFG, ®);
> rt2x00_set_field32(®, TX_RTY_CFG_SHORT_RTY_LIMIT, 15);
> rt2x00_set_field32(®, TX_RTY_CFG_LONG_RTY_LIMIT, 31);
> @@ -2413,7 +2556,6 @@ static int rt2800_init_bbp(struct rt2x00_dev *rt2x00dev)
> rt2800_bbp_write(rt2x00dev, 138, value);
> }
>
> -
This doesn't belong in here. Send it as separate cleanup please.
> for (i = 0; i < EEPROM_BBP_SIZE; i++) {
> rt2x00_eeprom_read(rt2x00dev, EEPROM_BBP_START + i, &eeprom);
>
> @@ -2996,13 +3138,6 @@ int rt2800_validate_eeprom(struct rt2x00_dev *rt2x00dev)
> default_lna_gain);
> rt2x00_eeprom_write(rt2x00dev, EEPROM_RSSI_A2, word);
>
> - rt2x00_eeprom_read(rt2x00dev, EEPROM_MAX_TX_POWER, &word);
> - if (rt2x00_get_field16(word, EEPROM_MAX_TX_POWER_24GHZ) == 0xff)
> - rt2x00_set_field16(&word, EEPROM_MAX_TX_POWER_24GHZ, MAX_G_TXPOWER);
> - if (rt2x00_get_field16(word, EEPROM_MAX_TX_POWER_5GHZ) == 0xff)
> - rt2x00_set_field16(&word, EEPROM_MAX_TX_POWER_5GHZ, MAX_A_TXPOWER);
> - rt2x00_eeprom_write(rt2x00dev, EEPROM_MAX_TX_POWER, word);
> -
> return 0;
> }
> EXPORT_SYMBOL_GPL(rt2800_validate_eeprom);
> @@ -3246,7 +3381,6 @@ int rt2800_probe_hw_mode(struct rt2x00_dev *rt2x00dev)
> char *default_power1;
> char *default_power2;
> unsigned int i;
> - unsigned short max_power;
> u16 eeprom;
>
> /*
> @@ -3371,26 +3505,22 @@ int rt2800_probe_hw_mode(struct rt2x00_dev *rt2x00dev)
>
> spec->channels_info = info;
>
> - rt2x00_eeprom_read(rt2x00dev, EEPROM_MAX_TX_POWER, &eeprom);
> - max_power = rt2x00_get_field16(eeprom, EEPROM_MAX_TX_POWER_24GHZ);
> + rt2x00_eeprom_read(rt2x00dev, EEPROM_EIRP_MAX_TX_POWER, &eeprom);
> default_power1 = rt2x00_eeprom_addr(rt2x00dev, EEPROM_TXPOWER_BG1);
> default_power2 = rt2x00_eeprom_addr(rt2x00dev, EEPROM_TXPOWER_BG2);
>
> for (i = 0; i < 14; i++) {
> - info[i].max_power = max_power;
> - info[i].default_power1 = TXPOWER_G_FROM_DEV(default_power1[i]);
> - info[i].default_power2 = TXPOWER_G_FROM_DEV(default_power2[i]);
Are the TXPOWER_G_FROM_DEV macros still used after removing the reference here?
> + info[i].default_power1 = default_power1[i];
> + info[i].default_power2 = default_power2[i];
> }
>
> if (spec->num_channels > 14) {
> - max_power = rt2x00_get_field16(eeprom, EEPROM_MAX_TX_POWER_5GHZ);
> default_power1 = rt2x00_eeprom_addr(rt2x00dev, EEPROM_TXPOWER_A1);
> default_power2 = rt2x00_eeprom_addr(rt2x00dev, EEPROM_TXPOWER_A2);
>
> for (i = 14; i < spec->num_channels; i++) {
> - info[i].max_power = max_power;
> - info[i].default_power1 = TXPOWER_A_FROM_DEV(default_power1[i]);
> - info[i].default_power2 = TXPOWER_A_FROM_DEV(default_power2[i]);
Are the TXPOWER_A_FROM_DEV macros still used after removing the reference here?
> + info[i].default_power1 = default_power1[i];
> + info[i].default_power2 = default_power2[i];
> }
> }
>
> diff --git a/drivers/net/wireless/rt2x00/rt2x00.h b/drivers/net/wireless/rt2x00/rt2x00.h
> index 7661e4f..96c2edd 100644
> --- a/drivers/net/wireless/rt2x00/rt2x00.h
> +++ b/drivers/net/wireless/rt2x00/rt2x00.h
> @@ -665,6 +665,7 @@ enum rt2x00_flags {
> */
> CONFIG_SUPPORT_HW_BUTTON,
> CONFIG_SUPPORT_HW_CRYPTO,
> + CONFIG_SUPPORT_POWER_LIMIT,
> DRIVER_SUPPORT_CONTROL_FILTERS,
> DRIVER_SUPPORT_CONTROL_FILTER_PSPOLL,
> DRIVER_SUPPORT_PRE_TBTT_INTERRUPT,
> @@ -682,6 +683,33 @@ enum rt2x00_flags {
> CONFIG_CHANNEL_HT40,
> };
>
> +/**
Please remove the second * as everywhere else in rt2x00.
> + * enum txpower_adjust_flags - txpower adjustment flags
> + * @TXPOWER_ACL_ADJUST: ALC adjustment periodically as temperature change
> + * @TXPOWER_CHANNEL_CONF_CHANGE: adjustmnet upon bands or bandwidth change
> + */
> +enum txpower_adjust_flags {
> + TXPOWER_ALC_ADJUST,
> + TXPOWER_CHANNEL_CONF_CHANGE,
> +};
> +
> +struct rt2x00_txpower_info {
> + unsigned long flags;
> + int txpower_limit;
> + int eirp_txpower[IEEE80211_NUM_BANDS];
> +
> +#define TXPOWER_CFG_NUM 5
> + /*
> + * Record default tx_power_cfg per data rate set for
> + * different band and bandwidth
> + */
> + u32 txpower_20M_2GHZ[TXPOWER_CFG_NUM];
> + u32 txpower_20M_5GHZ[TXPOWER_CFG_NUM];
> + u32 txpower_40M_2GHZ[TXPOWER_CFG_NUM];
> + u32 txpower_40M_5GHZ[TXPOWER_CFG_NUM];
> + u32 *default_txpower;
> +};
> +
> /*
> * rt2x00 device structure.
> */
> @@ -907,6 +935,12 @@ struct rt2x00_dev {
> * Protect the interrupt mask register.
> */
> spinlock_t irqmask_lock;
> +
> + /*
> + * TX power info is used for adjustment upon bandwidth, temperature,
> + * or power setting change.
> + */
> + struct rt2x00_txpower_info txpower_info;
> };
>
> /*
> diff --git a/drivers/net/wireless/rt2x00/rt2x00config.c b/drivers/net/wireless/rt2x00/rt2x00config.c
> index e7f67d5..b596525 100644
> --- a/drivers/net/wireless/rt2x00/rt2x00config.c
> +++ b/drivers/net/wireless/rt2x00/rt2x00config.c
> @@ -176,13 +176,27 @@ void rt2x00lib_config(struct rt2x00_dev *rt2x00dev,
>
> if (ieee80211_flags & IEEE80211_CONF_CHANGE_CHANNEL) {
> if (conf_is_ht40(conf)) {
> - __set_bit(CONFIG_CHANNEL_HT40, &rt2x00dev->flags);
> + if (!test_bit(CONFIG_CHANNEL_HT40, &rt2x00dev->flags)) {
> + __set_bit(CONFIG_CHANNEL_HT40, &rt2x00dev->flags);
> + __set_bit(TXPOWER_CHANNEL_CONF_CHANGE,
> + &rt2x00dev->txpower_info.flags);
> + }
> hw_value = rt2x00ht_center_channel(rt2x00dev, conf);
> } else {
> - __clear_bit(CONFIG_CHANNEL_HT40, &rt2x00dev->flags);
> + if (test_bit(CONFIG_CHANNEL_HT40, &rt2x00dev->flags)) {
> + __clear_bit(CONFIG_CHANNEL_HT40, &rt2x00dev->flags);
> + __set_bit(TXPOWER_CHANNEL_CONF_CHANGE,
> + &rt2x00dev->txpower_info.flags);
> + }
> hw_value = conf->channel->hw_value;
> }
>
> + if (rt2x00dev->curr_band != conf->channel->band) {
> + rt2x00dev->curr_band = conf->channel->band;
> + __set_bit(TXPOWER_CHANNEL_CONF_CHANGE,
> + &rt2x00dev->txpower_info.flags);
> + }
> +
> memcpy(&libconf.rf,
> &rt2x00dev->spec.channels[hw_value],
> sizeof(libconf.rf));
> @@ -204,7 +218,6 @@ void rt2x00lib_config(struct rt2x00_dev *rt2x00dev,
> if (ieee80211_flags & IEEE80211_CONF_CHANGE_CHANNEL)
> rt2x00link_reset_tuner(rt2x00dev, false);
>
> - rt2x00dev->curr_band = conf->channel->band;
> rt2x00dev->curr_freq = conf->channel->center_freq;
> rt2x00dev->tx_power = conf->power_level;
> rt2x00dev->short_retry = conf->short_frame_max_tx_count;
This part looks good to me.
I really appreciate your work ;) and hope you'll give the patch another
iteration based on the comments I made.
Thanks,
Helmut
More information about the users
mailing list