From: Felix Fietkau <nbd@openwrt.org>
To: Bala Shanmugam <bkamatch@qca.qualcomm.com>
Cc: linville@tuxdriver.com, linux-wireless@vger.kernel.org
Subject: Re: [PATCH v2] ath9k: Do not enable ANT diversity if ANT control bit is not set
Date: Tue, 30 Oct 2012 14:11:56 +0100 [thread overview]
Message-ID: <508FD21C.6080707@openwrt.org> (raw)
In-Reply-To: <1351595612-3566-1-git-send-email-bkamatch@qca.qualcomm.com>
On 2012-10-30 12:13 PM, Bala Shanmugam wrote:
> RvR test is not good when ANT control bit is not set so
> enable ANT diversity only when ANT control bit is set.
>
> Signed-off-by: Bala Shanmugam <bkamatch@qca.qualcomm.com>
I don't really like this patch; not only is the description very vague,
but it seems to me that the checks are done at the wrong place.
Further comments below...
> --- a/drivers/net/wireless/ath/ath9k/ar9003_phy.c
> +++ b/drivers/net/wireless/ath/ath9k/ar9003_phy.c
> @@ -1340,10 +1340,9 @@ static void ar9003_hw_antctrl_shared_chain_lnadiv(struct ath_hw *ah,
> regval = REG_READ(ah, AR_PHY_MC_GAIN_CTRL);
> regval &= (~AR_ANT_DIV_CTRL_ALL);
> regval |= (ant_div_ctl1 & 0x3f) << AR_ANT_DIV_CTRL_ALL_S;
> - regval &= ~AR_PHY_ANT_DIV_LNADIV;
> - regval |= ((ant_div_ctl1 >> 6) & 0x1) << AR_PHY_ANT_DIV_LNADIV_S;
>
> - if (enable)
> + if (enable &&
> + (ant_div_ctl1 & AR_EEP_ANT_DIV_ENABLE))
> regval |= AR_ANT_DIV_ENABLE;
>
> REG_WRITE(ah, AR_PHY_MC_GAIN_CTRL, regval);
The code should check much earlier (preferably somewhere in the eeprom
code) if diversity is available at all, and set a capability
accordingly, so that it doesn't even call this function with
enable==true if no diversity is available.
> @@ -1352,12 +1351,15 @@ static void ar9003_hw_antctrl_shared_chain_lnadiv(struct ath_hw *ah,
> regval &= ~AR_FAST_DIV_ENABLE;
> regval |= ((ant_div_ctl1 >> 7) & 0x1) << AR_FAST_DIV_ENABLE_S;
>
> - if (enable)
> + if (enable &&
> + (ant_div_ctl1 & AR_EEP_FAST_DIV_ENABLE))
> regval |= AR_FAST_DIV_ENABLE;
>
It would be much simpler to just remove the if statement and the line
below it. This would be equivalent to what you're doing because of this:
regval |= ((ant_div_ctl1 >> 7) & 0x1) << AR_FAST_DIV_ENABLE_S;
- Felix
prev parent reply other threads:[~2012-10-30 13:12 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-10-30 11:13 [PATCH v2] ath9k: Do not enable ANT diversity if ANT control bit is not set Bala Shanmugam
2012-10-30 13:11 ` Felix Fietkau [this message]
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=508FD21C.6080707@openwrt.org \
--to=nbd@openwrt.org \
--cc=bkamatch@qca.qualcomm.com \
--cc=linux-wireless@vger.kernel.org \
--cc=linville@tuxdriver.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).