[rt2x00-users] [PATCH 1/3] rt2x00: Fix rt2800 txpower setting to correct value

Helmut Schaa helmut.schaa at googlemail.com
Mon Jan 17 23:53:27 EST 2011


Hi Jay,

Am Montag, 17. Januar 2011 schrieb 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)
> >
> > What does the abbreviation ALC stand for?
> > As there is still a TODO on the list this series is meant as RFC?
> >
> 
> It is auto level control, chip need to adjust tx power value as temperature change.
> This patch do not include this compesation, it will be implemented future.

Ok, thanks. As this is still a TODO there's no need to mention it in the
commit log. If you want to include this information for the other developers
put it behind the "---" line, thus it won't end up in the commit log but is
available as part of the mail.

[...]

> > 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(&reg, 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).
> >
> 
> I know your method, but temeprate compesation will compesate tx power periodically.

Ok.

> and if dirver txpower calculation all put into rt2800_config_txpower, it will periodically call
> rt2800_config_txpower, so I calculate txpower base on rt2800_init_txpower, and can be
> used upon channel/band configuration or periodically used by temperature compesation.
> Any better idea??

What about storing a band_comp, a bw_comp and a temp_comp value globally in
rt2x00dev. band_comp and bw_comp get calculated on channel change (and stored)
while temp_comp gets calculated periodically (and stored). If either value
changes we can just call rt2800_config_txpower with the new compensation
values.

Even if we run that periodically this should be no performance bottleneck and
also the calculations are not as expensive as the register writes, and these
need to be done in every case.

So, I'm in favour of calculating the compensation values on the fly on each of
these events:
- channel change
- regulatory change
- periodic temperature compensation

And to reduce the amount of work to do periodically we can ignore txpower
config calls if none of the compensation values changed.

The benefits would be that we keep the number of per device variables low
(hence consuming less memory, which is not much in this case of course),
that we have only one code path manipulating the tx power settings and third
each compensation value can be computed independantly which should improve
code readability.

Do you see any other issues with this approach.

Thanks,
Helmut



More information about the users mailing list