Franky,
I was looking through the code and noticed the following in routine
wlc_phy_txpwrctrl_pwr_setup_nphy():
if (pi->sh->sromrev < 4) {
...
target_pwr_qtrdbm[0] = 13 * 4;
target_pwr_qtrdbm[1] = 13 * 4;
...
} else {
chan_freq_range = wlc_phy_get_chan_freq_range_nphy(pi, 0);
switch (chan_freq_range) {
case WL_CHAN_FREQ_RANGE_2G:
...
target_pwr_qtrdbm[0] =
pi->nphy_pwrctrl_info[0].max_pwr_2g;
target_pwr_qtrdbm[1] =
pi->nphy_pwrctrl_info[1].max_pwr_2g;
...
break;
case WL_CHAN_FREQ_RANGE_5GL:
...
target_pwr_qtrdbm[0] =
pi->nphy_pwrctrl_info[0].max_pwr_5gl;
target_pwr_qtrdbm[1] =
pi->nphy_pwrctrl_info[1].max_pwr_5gl;
...
break;
case WL_CHAN_FREQ_RANGE_5GM:
...
target_pwr_qtrdbm[0] =
pi->nphy_pwrctrl_info[0].max_pwr_5gm;
target_pwr_qtrdbm[1] =
pi->nphy_pwrctrl_info[1].max_pwr_5gm;
...
break;
case WL_CHAN_FREQ_RANGE_5GH:
...
target_pwr_qtrdbm[0] =
pi->nphy_pwrctrl_info[0].max_pwr_5gh;
target_pwr_qtrdbm[1] =
pi->nphy_pwrctrl_info[1].max_pwr_5gh;
...
break;
default:
...
target_pwr_qtrdbm[0] = 13 * 4;
target_pwr_qtrdbm[1] = 13 * 4;
...
break;
}
}
target_pwr_qtrdbm[0] = (s8) pi->tx_power_max;
target_pwr_qtrdbm[1] = (s8) pi->tx_power_max;
After going to some effort to customize the target_pwr_qtrdbm array depending on
the SPROM version and the particular channel being used, the array is
unconditionally overwritten in the end. Although gcc probably optimizes out the
statements that are not needed (I have not looked at the generated code.),
perhaps the code should be modified to make it clearer for human readers.
Thanks,
Larry
On 12/14/2011 05:19 AM, Larry Finger wrote:
> Franky,
>
> I was looking through the code and noticed the following in routine
> wlc_phy_txpwrctrl_pwr_setup_nphy():
>
> if (pi->sh->sromrev < 4) {
> ...
> target_pwr_qtrdbm[0] = 13 * 4;
> target_pwr_qtrdbm[1] = 13 * 4;
> ...
> } else {
> chan_freq_range = wlc_phy_get_chan_freq_range_nphy(pi, 0);
> switch (chan_freq_range) {
> case WL_CHAN_FREQ_RANGE_2G:
> ...
> target_pwr_qtrdbm[0] =
> pi->nphy_pwrctrl_info[0].max_pwr_2g;
> target_pwr_qtrdbm[1] =
> pi->nphy_pwrctrl_info[1].max_pwr_2g;
> ...
>
> break;
> case WL_CHAN_FREQ_RANGE_5GL:
> ...
> target_pwr_qtrdbm[0] =
> pi->nphy_pwrctrl_info[0].max_pwr_5gl;
> target_pwr_qtrdbm[1] =
> pi->nphy_pwrctrl_info[1].max_pwr_5gl;
> ...
> break;
> case WL_CHAN_FREQ_RANGE_5GM:
> ...
> target_pwr_qtrdbm[0] =
> pi->nphy_pwrctrl_info[0].max_pwr_5gm;
> target_pwr_qtrdbm[1] =
> pi->nphy_pwrctrl_info[1].max_pwr_5gm;
> ...
> break;
> case WL_CHAN_FREQ_RANGE_5GH:
> ...
> target_pwr_qtrdbm[0] =
> pi->nphy_pwrctrl_info[0].max_pwr_5gh;
> target_pwr_qtrdbm[1] =
> pi->nphy_pwrctrl_info[1].max_pwr_5gh;
> ...
> break;
> default:
> ...
> target_pwr_qtrdbm[0] = 13 * 4;
> target_pwr_qtrdbm[1] = 13 * 4;
> ...
> break;
> }
> }
>
> target_pwr_qtrdbm[0] = (s8) pi->tx_power_max;
> target_pwr_qtrdbm[1] = (s8) pi->tx_power_max;
>
> After going to some effort to customize the target_pwr_qtrdbm array depending on
> the SPROM version and the particular channel being used, the array is
> unconditionally overwritten in the end. Although gcc probably optimizes out the
> statements that are not needed (I have not looked at the generated code.),
> perhaps the code should be modified to make it clearer for human readers.
Yep. That looks pretty useless to me ;-) I will send a code-redux patch
for this. Feel free to share these kind of observations in a patch email
(so I can remain my lazy self and ack it ;-) ).
> Thanks,
>
> Larry
Thanks
AvS
On 12/14/2011 02:13 PM, Arend van Spriel wrote:
> Yep. That looks pretty useless to me ;-) I will send a code-redux patch
> for this. Feel free to share these kind of observations in a patch email
> (so I can remain my lazy self and ack it ;-) ).
OK. Actually, a patch would have been easier than a big edit on a
copy-and-paste. My only problem was whether the code that was specific to the
conditions was better, of if it is correct to always use the maximum power.
Larry