linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Bruno Randolf <br1@einfach.org>
To: linville@tuxdriver.com
Cc: ath5k-devel@lists.ath5k.org, linux-wireless@vger.kernel.org
Subject: [PATCH 10/13] ath5k: fix I/Q calibration (for real)
Date: Tue, 09 Mar 2010 16:56:05 +0900	[thread overview]
Message-ID: <20100309075605.30616.63221.stgit@void> (raw)
In-Reply-To: <20100309075124.30616.40896.stgit@void>

I/Q calibration was completely broken, resulting in a high number of CRC errors
on received packets. before i could see around 10% to 20% CRC errors, with this
patch they are between 0% and 3%.

1.) the removal of the mask in commit "ath5k: Fix I/Q calibration
(f1cf2dbd0f798b71b1590e7aca6647f2caef1649)" resulted in no mask beeing used
when writing the I/Q values into the register. additional errors in the
calculation of the values (see 2.) resulted too high numbers, exceeding the
masks, so wrong values like 0xfffffffe were written. to be safe we should
always use the bitmask when writing parts of a register.

2.) using a (s32) cast for q_coff is a wrong conversion to signed, since we
convert to a signed value later by substracting 128. this resulted in too low
numbers for Q many times, which were limited to -16 by the boundary check later
on.

3.) checked everything against the HAL sources and took over comments and minor
optimizations from there.

4.) we can't use ENABLE_BITS when we want to write a number (the number can
contain zeros). also always write the correction values first and set ENABLE
bit last, like the HAL does.

Cc: stable@kernel.org

Signed-off-by: Bruno Randolf <br1@einfach.org>
---
 drivers/net/wireless/ath/ath5k/phy.c |   37 +++++++++++++++++-----------------
 drivers/net/wireless/ath/ath5k/reg.h |    1 +
 2 files changed, 20 insertions(+), 18 deletions(-)

diff --git a/drivers/net/wireless/ath/ath5k/phy.c b/drivers/net/wireless/ath/ath5k/phy.c
index 3fa4f4d..95cebd6 100644
--- a/drivers/net/wireless/ath/ath5k/phy.c
+++ b/drivers/net/wireless/ath/ath5k/phy.c
@@ -1386,38 +1386,39 @@ static int ath5k_hw_rf511x_calibrate(struct ath5k_hw *ah,
 		goto done;
 
 	/* Calibration has finished, get the results and re-run */
+
+	/* work around empty results which can apparently happen on 5212 */
 	for (i = 0; i <= 10; i++) {
 		iq_corr = ath5k_hw_reg_read(ah, AR5K_PHY_IQRES_CAL_CORR);
 		i_pwr = ath5k_hw_reg_read(ah, AR5K_PHY_IQRES_CAL_PWR_I);
 		q_pwr = ath5k_hw_reg_read(ah, AR5K_PHY_IQRES_CAL_PWR_Q);
+		ATH5K_DBG_UNLIMIT(ah->ah_sc, ATH5K_DEBUG_CALIBRATE,
+			"iq_corr:%x i_pwr:%x q_pwr:%x", iq_corr, i_pwr, q_pwr);
+		if (i_pwr && q_pwr)
+			break;
 	}
 
 	i_coffd = ((i_pwr >> 1) + (q_pwr >> 1)) >> 7;
 	q_coffd = q_pwr >> 7;
 
-	/* No correction */
-	if (i_coffd == 0 || q_coffd == 0)
+	/* protect against divide by 0 and loss of sign bits */
+	if (i_coffd == 0 || q_coffd < 2)
 		goto done;
 
-	i_coff = ((-iq_corr) / i_coffd);
+	i_coff = (-iq_corr) / i_coffd;
+	i_coff = clamp(i_coff, -32, 31); /* signed 6 bit */
 
-	/* Boundary check */
-	if (i_coff > 31)
-		i_coff = 31;
-	if (i_coff < -32)
-		i_coff = -32;
+	q_coff = (i_pwr / q_coffd) - 128;
+	q_coff = clamp(q_coff, -16, 15); /* signed 5 bit */
 
-	q_coff = (((s32)i_pwr / q_coffd) - 128);
+	ATH5K_DBG_UNLIMIT(ah->ah_sc, ATH5K_DEBUG_CALIBRATE,
+			"new I:%d Q:%d (i_coffd:%x q_coffd:%x)",
+			i_coff, q_coff, i_coffd, q_coffd);
 
-	/* Boundary check */
-	if (q_coff > 15)
-		q_coff = 15;
-	if (q_coff < -16)
-		q_coff = -16;
-
-	/* Commit new I/Q value */
-	AR5K_REG_ENABLE_BITS(ah, AR5K_PHY_IQ, AR5K_PHY_IQ_CORR_ENABLE |
-		((u32)q_coff) | ((u32)i_coff << AR5K_PHY_IQ_CORR_Q_I_COFF_S));
+	/* Commit new I/Q values (set enable bit last to match HAL sources) */
+	AR5K_REG_WRITE_BITS(ah, AR5K_PHY_IQ, AR5K_PHY_IQ_CORR_Q_I_COFF, i_coff);
+	AR5K_REG_WRITE_BITS(ah, AR5K_PHY_IQ, AR5K_PHY_IQ_CORR_Q_Q_COFF, q_coff);
+	AR5K_REG_ENABLE_BITS(ah, AR5K_PHY_IQ, AR5K_PHY_IQ_CORR_ENABLE);
 
 	/* Re-enable calibration -if we don't we'll commit
 	 * the same values again and again */
diff --git a/drivers/net/wireless/ath/ath5k/reg.h b/drivers/net/wireless/ath/ath5k/reg.h
index 4cb9c5d..1464f89 100644
--- a/drivers/net/wireless/ath/ath5k/reg.h
+++ b/drivers/net/wireless/ath/ath5k/reg.h
@@ -2187,6 +2187,7 @@
  */
 #define	AR5K_PHY_IQ			0x9920			/* Register Address */
 #define	AR5K_PHY_IQ_CORR_Q_Q_COFF	0x0000001f	/* Mask for q correction info */
+#define	AR5K_PHY_IQ_CORR_Q_Q_COFF_S	0
 #define	AR5K_PHY_IQ_CORR_Q_I_COFF	0x000007e0	/* Mask for i correction info */
 #define	AR5K_PHY_IQ_CORR_Q_I_COFF_S	5
 #define	AR5K_PHY_IQ_CORR_ENABLE		0x00000800	/* Enable i/q correction */


  parent reply	other threads:[~2010-03-09  7:56 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-03-09  7:55 [PATCH 00/13] all my ath5k patches Bruno Randolf
2010-03-09  7:55 ` [PATCH 01/13] ath5k: add antenna statistics and debugfs file for antenna settings Bruno Randolf
2010-03-09 12:15   ` Nick Kossifidis
2010-03-09  7:55 ` [PATCH 02/13] ath5k: use fixed antenna for tx descriptors Bruno Randolf
2010-03-09 12:16   ` Nick Kossifidis
2010-03-09  7:55 ` [PATCH 03/13] ath5k: preserve antenna settings Bruno Randolf
2010-03-09 12:16   ` Nick Kossifidis
2010-03-09  7:55 ` [PATCH 04/13] ath5k: fix TSF reset Bruno Randolf
2010-03-09 12:17   ` Nick Kossifidis
2010-03-09  7:55 ` [PATCH 05/13] ath5k: remove double opmode definition Bruno Randolf
2010-03-09 12:17   ` Nick Kossifidis
2010-03-09  7:55 ` [PATCH 06/13] ath5k: remove ah_magic Bruno Randolf
2010-03-09 12:18   ` Nick Kossifidis
2010-03-09  7:55 ` [PATCH 07/13] ath5k: remove ah_mac_revision Bruno Randolf
2010-03-09 12:18   ` Nick Kossifidis
2010-03-09  7:55 ` [PATCH 08/13] ath5k: remove ah_gpio_npins Bruno Randolf
2010-03-09 12:19   ` Nick Kossifidis
2010-03-09  7:56 ` [PATCH 09/13] ath5k: add debugfs file frameerrors Bruno Randolf
2010-03-09 12:21   ` Nick Kossifidis
2010-03-09  7:56 ` Bruno Randolf [this message]
2010-03-09 12:22   ` [PATCH 10/13] ath5k: fix I/Q calibration (for real) Nick Kossifidis
2010-03-09  7:56 ` [PATCH 11/13] ath5k: read eeprom IQ calibration values correctly for G mode Bruno Randolf
2010-03-09 12:22   ` Nick Kossifidis
2010-03-09  7:56 ` [PATCH 12/13] ath5k: IQ calibration for AR5211 is slightly different Bruno Randolf
2010-03-09 12:23   ` Nick Kossifidis
2010-03-09  7:56 ` [PATCH 13/13] ath5k: Minor EEPROM documentation updates Bruno Randolf
2010-03-09 14:33 ` [PATCH 00/13] all my ath5k patches John W. Linville

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=20100309075605.30616.63221.stgit@void \
    --to=br1@einfach.org \
    --cc=ath5k-devel@lists.ath5k.org \
    --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).