From: "Arend van Spriel" <arend@broadcom.com>
To: "Larry Finger" <Larry.Finger@lwfinger.net>
Cc: "Franky (Zhenhui) Lin" <frankyl@broadcom.com>,
wireless <linux-wireless@vger.kernel.org>
Subject: Re: Some funny code in brcmsmac
Date: Wed, 14 Dec 2011 21:13:30 +0100 [thread overview]
Message-ID: <4EE9036A.7000309@broadcom.com> (raw)
In-Reply-To: <4EE823C0.7080205@lwfinger.net>
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
next prev parent reply other threads:[~2011-12-14 20:13 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-12-14 4:19 Some funny code in brcmsmac Larry Finger
2011-12-14 20:13 ` Arend van Spriel [this message]
2011-12-14 21:22 ` Larry Finger
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4EE9036A.7000309@broadcom.com \
--to=arend@broadcom.com \
--cc=Larry.Finger@lwfinger.net \
--cc=frankyl@broadcom.com \
--cc=linux-wireless@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.