From: Jan Ceuleers <jan.ceuleers@computer.org>
To: "Devendra.Naga" <devendra.aaru@gmail.com>
Cc: Larry.Finger@lwfinger.net, chaoming_li@realsil.com.cn,
linville@tuxdriver.com, paul.gortmaker@windriver.com,
joe@perches.com, linux-wireless@vger.kernel.org,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/1] wireless: move ifs' in _rtl92c_store_pwrIndex_diffrate_offset into elseif statements
Date: Sun, 05 Feb 2012 09:12:38 +0100 [thread overview]
Message-ID: <4F2E39F6.70803@computer.org> (raw)
In-Reply-To: <1327906245-8200-1-git-send-email-devendra.aaru@gmail.com>
On 01/30/2012 07:50 AM, Devendra.Naga wrote:
> From: "Devendra.Naga"<devendra.aaru@gmail.com>
>
> The regaddr is compared against different rates in each case and the powerlevels are set
> accordingly. as this can match for the first if and we still check for other powerlevels too,
> use else if ratherthan using if statements
> Signed-off-by: Devendra.Naga<devendra.aaru@gmail.com>
If this is correct, and I'm not saying it is or isn't, would it not be
clearer to turn this whole section into a switch statement? The cases
that requiring fiddling with the bitmask would need to be dealt with in
the default: case.
Why was it written as a series of if statements to begin with: can (or
could) any of these if statements change the value of regaddr such that
later ifs evaluate differently? From reading only the patch the answer
seems to be "no", so that looks good.
Or what about turning this into a data-driven section? It looks like
values of regaddr are being translated into offsets into
rtlphy->MCS_TXPWR[rtlphy->pwrgroup_cnt]. Can this be done with a lookup
table?
> ---
> drivers/net/wireless/rtlwifi/rtl8192c/phy_common.c | 47 +++++++------------
> 1 files changed, 17 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/net/wireless/rtlwifi/rtl8192c/phy_common.c b/drivers/net/wireless/rtlwifi/rtl8192c/phy_common.c
> index 22e998d..cb11e5a 100644
> --- a/drivers/net/wireless/rtlwifi/rtl8192c/phy_common.c
> +++ b/drivers/net/wireless/rtlwifi/rtl8192c/phy_common.c
> @@ -260,106 +260,93 @@ void _rtl92c_store_pwrIndex_diffrate_offset(struct ieee80211_hw *hw,
> "MCSTxPowerLevelOriginalOffset[%d][0] = 0x%x\n",
> rtlphy->pwrgroup_cnt,
> rtlphy->MCS_TXPWR[rtlphy->pwrgroup_cnt][0]);
> - }
> - if (regaddr == RTXAGC_A_RATE54_24) {
> + } else if (regaddr == RTXAGC_A_RATE54_24) {
> rtlphy->MCS_TXPWR[rtlphy->pwrgroup_cnt][1] = data;
> RT_TRACE(rtlpriv, COMP_INIT, DBG_TRACE,
> "MCSTxPowerLevelOriginalOffset[%d][1] = 0x%x\n",
> rtlphy->pwrgroup_cnt,
> rtlphy->MCS_TXPWR[rtlphy->pwrgroup_cnt][1]);
> - }
> - if (regaddr == RTXAGC_A_CCK1_MCS32) {
> + } else if (regaddr == RTXAGC_A_CCK1_MCS32) {
> rtlphy->MCS_TXPWR[rtlphy->pwrgroup_cnt][6] = data;
> RT_TRACE(rtlpriv, COMP_INIT, DBG_TRACE,
> "MCSTxPowerLevelOriginalOffset[%d][6] = 0x%x\n",
> rtlphy->pwrgroup_cnt,
> rtlphy->MCS_TXPWR[rtlphy->pwrgroup_cnt][6]);
> - }
> - if (regaddr == RTXAGC_B_CCK11_A_CCK2_11&& bitmask == 0xffffff00) {
> + } else if (regaddr == RTXAGC_B_CCK11_A_CCK2_11&&
> + bitmask == 0xffffff00) {
> rtlphy->MCS_TXPWR[rtlphy->pwrgroup_cnt][7] = data;
> RT_TRACE(rtlpriv, COMP_INIT, DBG_TRACE,
> "MCSTxPowerLevelOriginalOffset[%d][7] = 0x%x\n",
> rtlphy->pwrgroup_cnt,
> rtlphy->MCS_TXPWR[rtlphy->pwrgroup_cnt][7]);
> - }
> - if (regaddr == RTXAGC_A_MCS03_MCS00) {
> + } else if (regaddr == RTXAGC_A_MCS03_MCS00) {
> rtlphy->MCS_TXPWR[rtlphy->pwrgroup_cnt][2] = data;
> RT_TRACE(rtlpriv, COMP_INIT, DBG_TRACE,
> "MCSTxPowerLevelOriginalOffset[%d][2] = 0x%x\n",
> rtlphy->pwrgroup_cnt,
> rtlphy->MCS_TXPWR[rtlphy->pwrgroup_cnt][2]);
> - }
> - if (regaddr == RTXAGC_A_MCS07_MCS04) {
> + } else if (regaddr == RTXAGC_A_MCS07_MCS04) {
> rtlphy->MCS_TXPWR[rtlphy->pwrgroup_cnt][3] = data;
> RT_TRACE(rtlpriv, COMP_INIT, DBG_TRACE,
> "MCSTxPowerLevelOriginalOffset[%d][3] = 0x%x\n",
> rtlphy->pwrgroup_cnt,
> rtlphy->MCS_TXPWR[rtlphy->pwrgroup_cnt][3]);
> - }
> - if (regaddr == RTXAGC_A_MCS11_MCS08) {
> + } else if (regaddr == RTXAGC_A_MCS11_MCS08) {
> rtlphy->MCS_TXPWR[rtlphy->pwrgroup_cnt][4] = data;
> RT_TRACE(rtlpriv, COMP_INIT, DBG_TRACE,
> "MCSTxPowerLevelOriginalOffset[%d][4] = 0x%x\n",
> rtlphy->pwrgroup_cnt,
> rtlphy->MCS_TXPWR[rtlphy->pwrgroup_cnt][4]);
> - }
> - if (regaddr == RTXAGC_A_MCS15_MCS12) {
> + } else if (regaddr == RTXAGC_A_MCS15_MCS12) {
> rtlphy->MCS_TXPWR[rtlphy->pwrgroup_cnt][5] = data;
> RT_TRACE(rtlpriv, COMP_INIT, DBG_TRACE,
> "MCSTxPowerLevelOriginalOffset[%d][5] = 0x%x\n",
> rtlphy->pwrgroup_cnt,
> rtlphy->MCS_TXPWR[rtlphy->pwrgroup_cnt][5]);
> - }
> - if (regaddr == RTXAGC_B_RATE18_06) {
> + } else if (regaddr == RTXAGC_B_RATE18_06) {
> rtlphy->MCS_TXPWR[rtlphy->pwrgroup_cnt][8] = data;
> RT_TRACE(rtlpriv, COMP_INIT, DBG_TRACE,
> "MCSTxPowerLevelOriginalOffset[%d][8] = 0x%x\n",
> rtlphy->pwrgroup_cnt,
> rtlphy->MCS_TXPWR[rtlphy->pwrgroup_cnt][8]);
> - }
> - if (regaddr == RTXAGC_B_RATE54_24) {
> + } else if (regaddr == RTXAGC_B_RATE54_24) {
> rtlphy->MCS_TXPWR[rtlphy->pwrgroup_cnt][9] = data;
> RT_TRACE(rtlpriv, COMP_INIT, DBG_TRACE,
> "MCSTxPowerLevelOriginalOffset[%d][9] = 0x%x\n",
> rtlphy->pwrgroup_cnt,
> rtlphy->MCS_TXPWR[rtlphy->pwrgroup_cnt][9]);
> - }
> - if (regaddr == RTXAGC_B_CCK1_55_MCS32) {
> + } else if (regaddr == RTXAGC_B_CCK1_55_MCS32) {
> rtlphy->MCS_TXPWR[rtlphy->pwrgroup_cnt][14] = data;
> RT_TRACE(rtlpriv, COMP_INIT, DBG_TRACE,
> "MCSTxPowerLevelOriginalOffset[%d][14] = 0x%x\n",
> rtlphy->pwrgroup_cnt,
> rtlphy->MCS_TXPWR[rtlphy->pwrgroup_cnt][14]);
> - }
> - if (regaddr == RTXAGC_B_CCK11_A_CCK2_11&& bitmask == 0x000000ff) {
> + } else if (regaddr == RTXAGC_B_CCK11_A_CCK2_11&&
> + bitmask == 0x000000ff) {
> rtlphy->MCS_TXPWR[rtlphy->pwrgroup_cnt][15] = data;
> RT_TRACE(rtlpriv, COMP_INIT, DBG_TRACE,
> "MCSTxPowerLevelOriginalOffset[%d][15] = 0x%x\n",
> rtlphy->pwrgroup_cnt,
> rtlphy->MCS_TXPWR[rtlphy->pwrgroup_cnt][15]);
> - }
> - if (regaddr == RTXAGC_B_MCS03_MCS00) {
> + } else if (regaddr == RTXAGC_B_MCS03_MCS00) {
> rtlphy->MCS_TXPWR[rtlphy->pwrgroup_cnt][10] = data;
> RT_TRACE(rtlpriv, COMP_INIT, DBG_TRACE,
> "MCSTxPowerLevelOriginalOffset[%d][10] = 0x%x\n",
> rtlphy->pwrgroup_cnt,
> rtlphy->MCS_TXPWR[rtlphy->pwrgroup_cnt][10]);
> - }
> - if (regaddr == RTXAGC_B_MCS07_MCS04) {
> + } else if (regaddr == RTXAGC_B_MCS07_MCS04) {
> rtlphy->MCS_TXPWR[rtlphy->pwrgroup_cnt][11] = data;
> RT_TRACE(rtlpriv, COMP_INIT, DBG_TRACE,
> "MCSTxPowerLevelOriginalOffset[%d][11] = 0x%x\n",
> rtlphy->pwrgroup_cnt,
> rtlphy->MCS_TXPWR[rtlphy->pwrgroup_cnt][11]);
> - }
> - if (regaddr == RTXAGC_B_MCS11_MCS08) {
> + } else if (regaddr == RTXAGC_B_MCS11_MCS08) {
> rtlphy->MCS_TXPWR[rtlphy->pwrgroup_cnt][12] = data;
> RT_TRACE(rtlpriv, COMP_INIT, DBG_TRACE,
> "MCSTxPowerLevelOriginalOffset[%d][12] = 0x%x\n",
> rtlphy->pwrgroup_cnt,
> rtlphy->MCS_TXPWR[rtlphy->pwrgroup_cnt][12]);
> - }
> - if (regaddr == RTXAGC_B_MCS15_MCS12) {
> + } else if (regaddr == RTXAGC_B_MCS15_MCS12) {
> rtlphy->MCS_TXPWR[rtlphy->pwrgroup_cnt][13] = data;
> RT_TRACE(rtlpriv, COMP_INIT, DBG_TRACE,
> "MCSTxPowerLevelOriginalOffset[%d][13] = 0x%x\n",
next prev parent reply other threads:[~2012-02-05 8:13 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-01-30 6:50 [PATCH 1/1] wireless: move ifs' in _rtl92c_store_pwrIndex_diffrate_offset into elseif statements Devendra.Naga
2012-01-30 23:38 ` Larry Finger
2012-02-05 8:12 ` Jan Ceuleers [this message]
2012-02-05 16:05 ` Larry Finger
2012-02-06 11:26 ` David Laight
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=4F2E39F6.70803@computer.org \
--to=jan.ceuleers@computer.org \
--cc=Larry.Finger@lwfinger.net \
--cc=chaoming_li@realsil.com.cn \
--cc=devendra.aaru@gmail.com \
--cc=joe@perches.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-wireless@vger.kernel.org \
--cc=linville@tuxdriver.com \
--cc=netdev@vger.kernel.org \
--cc=paul.gortmaker@windriver.com \
/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 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).