linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Some funny code in brcmsmac
@ 2011-12-14  4:19 Larry Finger
  2011-12-14 20:13 ` Arend van Spriel
  0 siblings, 1 reply; 3+ messages in thread
From: Larry Finger @ 2011-12-14  4:19 UTC (permalink / raw)
  To: Franky Lin, wireless

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: Some funny code in brcmsmac
  2011-12-14  4:19 Some funny code in brcmsmac Larry Finger
@ 2011-12-14 20:13 ` Arend van Spriel
  2011-12-14 21:22   ` Larry Finger
  0 siblings, 1 reply; 3+ messages in thread
From: Arend van Spriel @ 2011-12-14 20:13 UTC (permalink / raw)
  To: Larry Finger; +Cc: Franky (Zhenhui) Lin, wireless

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


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: Some funny code in brcmsmac
  2011-12-14 20:13 ` Arend van Spriel
@ 2011-12-14 21:22   ` Larry Finger
  0 siblings, 0 replies; 3+ messages in thread
From: Larry Finger @ 2011-12-14 21:22 UTC (permalink / raw)
  To: Arend van Spriel; +Cc: Franky (Zhenhui) Lin, wireless

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2011-12-14 21:22 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-12-14  4:19 Some funny code in brcmsmac Larry Finger
2011-12-14 20:13 ` Arend van Spriel
2011-12-14 21:22   ` Larry Finger

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).