[rt2x00-users] [PATCH] rt2x00: Fix rt2800 twpower setting to correct value

[RA-Jay Hung]

> > 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)
>
> Well the patch doesn't apply to my rt2x00.git tree (same as your
> previous patches). I guess either your rt2x00.git checkout is too old,
> or that your mail client is corrupting your patches. Can you please
> check? Otherwise I have to keep manually applying your patches...

I remember I have git fetch to sync with latest code before sending this patch. Not sure If my mail client have problem.
I will resend again base on code base you update today.


> > diff --git a/drivers/net/wireless/rt2x00/rt2800lib.c
> b/drivers/net/wireless/rt2x00/rt2800lib.c
> > index b7d91d5..4ebd601 100644
> > --- a/drivers/net/wireless/rt2x00/rt2800lib.c
> > +++ b/drivers/net/wireless/rt2x00/rt2800lib.c
> > @@ -939,6 +939,174 @@ static void rt2800_init_led(struct rt2x00_dev
> *rt2x00dev,
> >  }
> >  #endif /* CONFIG_RT2X00_LIB_LEDS */
> >
> > +static void rt2800_txpower_delta(struct rt2x00_dev *rt2x00dev,
> > +                        u8 txpow_delta, u32 *txpow_cfg_20M, u32
> *txpow_cfg_40M)
>
> please write 'txpow' as txpower', there isn't a need to abbreviate the names.
>
Will update
> > +{
> > +       int i;
> > +       u8 comp_en;
> > +       u8 comp_type;
> > +       u8 comp_value;
> > +
> > +       comp_en = rt2x00_get_field8(txpow_delta,
> >
> +                                    EEPROM_TXPOWER_DELTA_ENA
> BLE);
> > +       if (comp_en) {
> > +               comp_type = rt2x00_get_field8(txpow_delta,
> >
> +                                              EEPROM_TXPOWER_
> DELTA_TYPE);
> > +               comp_value = rt2x00_get_field8(txpow_delta,
> >
> +                                              EEPROM_TXPOWER_
> DELTA_VALUE);
> > +
> > +               for (i = 0; i < TXPOW_CFG_NUM; i++) {
> > +                       if (comp_type)
> > +                               txpow_cfg_40M[i] = txpow_cfg_20M[i] +
> comp_value;
> > +                       else
> > +                               txpow_cfg_40M[i] = txpow_cfg_20M[i] -
> comp_value;
>
> Perhaps we can simplify this, it a bit by doing something like:
>
>    comp_type = ...
>    comp_value = ...
>
>   if (!comp_type)
>      comp_value = -comp_value;
>
Will update
>   for (i = 0; i < TXPOW_CFG_NUM; i++)
>     txpow_cfg_40M[i] = txpow_cfg_20M[i] + comp_value;
> >        CONFIG_SUPPORT_HW_BUTTON,
> >        CONFIG_SUPPORT_HW_CRYPTO,
> > +       CONFIG_SUPPORT_POW_LIMIT,
>
> CONFIG_SUPPORT_POWER_LIMIT
Will update, too
> > +/**
> > + * enum txpwr_adjust_flags - txpower adjustment flags
> > + * @TXPWR_ACL_ADJUST: ALC adjustment periodically as temperature
> change
> > + * @TXPWR_CHANNEL_CONF_CHANGE: adjustmnet upon bands or
> bandwidth change
> > + */
> > +enum txpow_adjust_flags {
> > +       TXPOW_ALC_ADJUST,
> > +       TXPOW_CHANNEL_CONF_CHANGE,
> > +};
>
> txpower_adjust_flags
> TXPOWER_ALC_ADJUST
> TXPOWER_CHANNEL_CONF_CHANGE
>
> >  /*
> > diff --git a/drivers/net/wireless/rt2x00/rt2x00config.c
> b/drivers/net/wireless/rt2x00/rt2x00config.c
> > index e7f67d5..5bb37d7 100644
> > --- a/drivers/net/wireless/rt2x00/rt2x00config.c
> > +++ b/drivers/net/wireless/rt2x00/rt2x00config.c
> > @@ -176,13 +176,25 @@ 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(TXPOW_CHANNEL_CONF_CH
> ANGE, &rt2x00dev->txpow_info.flags);
>
> We are handling the configuration handlers already with a changed flag, which
> we send to the driver. So why have this separate flag? When the driver
> receives the
> command to update the TXPower using this method, then
> TXPOW_CHANNEL_CONF_CHANGE
> will also be true. So we don't need this second flag.

IEEE80211_CONF_CHANGE_CHANNEL do not mean need to update tx power.
Only when bandwidth or band change need to set tx power

>
> > -       rt2x00dev->curr_band = conf->channel->band;
> > -       rt2x00dev->curr_freq = conf->channel->center_freq;

I move this two line before called rt2x00dev->ops->lib->config, and only when band change, so curr_band seems OK for each device. I will fix curr_freq setting, seems it will not be set on non-rt2800 device.

> This will break every non-rt2800 driver.

Jay
CONFIDENTIALITY STATEMENT : The information, attachments and any rights attaching in this e-mail are confidential and privileged; it is intended only for the individual or entity named as the recipient hereof.Any disclosure, copying, distribution, dissemination or use of the contents of this e-mail by persons other than the intended recipient is STRICTLY PROHIBITED and may violate applicable laws.If you have received this e-mail in error, please delete the original message and notify us by return email or collect call immediately. Thank you.