[PATCH wireless-next 0/5] b43: N-PHY: fix calibrations on rev7+ (BCM43228)

[PATCH wireless-next 0/5] b43: N-PHY: fix calibrations on rev7+ (BCM43228)

[Luca Casati]

On a BCM43228 (14e4:4359, N-PHY rev 16, radio 2057 rev 9) b43
associates but transmits poorly: high retry counts and low rates.
Comparing mmiotraces of b43 and the proprietary wl driver
(6.30.223.271) on the same card, together with brcmsmac, turned up
several bugs in the N-PHY calibration code:

 1. The TX tone sample table was loaded as all zeros, so every
    tone-based calibration ran without a signal.
 2. TX IQ/LO calibration: inverted busy poll, reversed final copy,
    and missing rev7+ cleanup that left TX dead after a cal.
 3. RSSI calibration: wrong VCM written on rev7+, wrong distance
    formula, wrong rounding of negative offsets.
 4. 2057 RC calibration: the first run was polled before it started,
    leaving SCAP at 0 and the TX low-pass filter detuned.
 5. A missing field in the rev7 TX_PU RF override.

After these, the register writes for the cals match wl's on this card.

Patches 4 and 5 and the cleanup parts of patch 2 only affect rev7+.
Patch 1, the poll and final-copy fixes in patch 2, and the
distance/rounding fixes in patch 3 also change code that runs on older
N-PHYs (rev3+); I could only test rev 16, so testing on other N-PHY
cards would be welcome.

Tested on the BCM43228 only (2.4 GHz, 11g/HT20), on v7.0; the series
also applies to wireless-next. Throughput to the internet went from
about 5.7/4.1 to 11.1/5.2 Mbit/s down/up, with fewer TX retries. That
number is limited by the line and noisy, so take it as a rough
indication; wl reaches 21-32 Mbit/s down on the same setup, so there
is more to do (PAPD and TX power control on rev7+).

Comments and changelogs refer to wl where the fix was checked against
its register writes. If citing the proprietary driver is not
acceptable here, I am happy to reword them.

The bugs were found and the fixes drafted with the help of an AI
coding assistant (Claude Opus 5.5), working from the mmiotrace diffs
between b43 and wl and from brcmsmac. I reviewed and tested every
patch on the hardware above.

Luca Casati (5):
  b43: N-PHY: fix the TX tone sample table
  b43: N-PHY: fix TX IQ/LO calibration
  b43: N-PHY: fix rev3+ RSSI calibration
  b43: N-PHY: wait for the first 2057 RC calibration run
  b43: N-PHY: add the missing override to the rev7 TX_PU command

 drivers/net/wireless/broadcom/b43/phy_n.c | 120 +++++++++++++++++++---
 1 file changed, 106 insertions(+), 14 deletions(-)


base-commit: 028ef9c96e96197026887c0f092424679298aae8
-- 
2.43.0

[PATCH wireless-next 1/5] b43: N-PHY: fix the TX tone sample table

[Luca Casati]

b43_nphy_load_samples() masked with (0x3FF << 10) instead of shifting
the masked I value, and b43_nphy_gen_load_samples() fed
cordic_calc_iq() a 16.16 fixed-point angle (CORDIC_FIXED() of a value
already shifted by 16) held in a u16, while the function takes whole
degrees. Together the sample table ended up all zeros, so every
calibration that plays a test tone (TX IQ/LO, RX IQ) ran without a
signal.

Compute the rotation in whole degrees as brcmsmac does and fix the
precedence.

Tested on a BCM43228 (N-PHY rev 16, radio 2057 rev 9).

Found and fixed with the help of an AI coding assistant (Claude Opus 5.5),
by comparing mmiotraces of b43 and wl on the same card.

Signed-off-by: Luca Casati <luk00casati@gmail.com>
---
 drivers/net/wireless/broadcom/b43/phy_n.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/broadcom/b43/phy_n.c b/drivers/net/wireless/broadcom/b43/phy_n.c
index bbc30cbad..66565c14e 100644
--- a/drivers/net/wireless/broadcom/b43/phy_n.c
+++ b/drivers/net/wireless/broadcom/b43/phy_n.c
@@ -1515,7 +1515,7 @@ static int b43_nphy_load_samples(struct b43_wldev *dev,
 		b43_nphy_stay_in_carrier_search(dev, 1);
 
 	for (i = 0; i < len; i++) {
-		data[i] = (samples[i].i & 0x3FF << 10);
+		data[i] = (samples[i].i & 0x3FF) << 10;
 		data[i] |= samples[i].q & 0x3FF;
 	}
 	b43_ntab_write_bulk(dev, B43_NTAB32(17, 0), len, data);
@@ -1531,7 +1531,8 @@ static u16 b43_nphy_gen_load_samples(struct b43_wldev *dev, u32 freq, u16 max,
 					bool test)
 {
 	int i;
-	u16 bw, len, rot, angle;
+	u16 bw, len;
+	s32 rot, angle;		/* whole degrees, as cordic_calc_iq() wants */
 	struct cordic_iq *samples;
 
 	bw = b43_is_40mhz(dev) ? 40 : 20;
@@ -1554,11 +1555,11 @@ static u16 b43_nphy_gen_load_samples(struct b43_wldev *dev, u32 freq, u16 max,
 		b43err(dev->wl, "allocation for samples generation failed\n");
 		return 0;
 	}
-	rot = (((freq * 36) / bw) << 16) / 100;
+	rot = ((freq * 36) / bw) / 100;
 	angle = 0;
 
 	for (i = 0; i < len; i++) {
-		samples[i] = cordic_calc_iq(CORDIC_FIXED(angle));
+		samples[i] = cordic_calc_iq(angle);
 		angle += rot;
 		samples[i].q = CORDIC_FLOAT(samples[i].q * max);
 		samples[i].i = CORDIC_FLOAT(samples[i].i * max);
-- 
2.43.0

[PATCH wireless-next 2/5] b43: N-PHY: fix TX IQ/LO calibration

[Luca Casati]

Several bugs kept the TX IQ/LO calibration from working, and on rev7+
it left TX dead afterwards:

- The busy poll on IQLOCAL_CMD was inverted: bits 15:14 are set while a
  step runs, so it stopped waiting as soon as the step started.
- The final copy was reversed: it wrote the stale buffer over the best
  coefficients (table 15 at 96) and read the active ones (at 80), so the
  result was never applied.
- TxCalPhyCleanup lacked the rev7+ part of brcmsmac's
  wlc_phy_txcal_phycleanup_nphy(): the 0x80 and 0x8 (PA power-up) RF
  overrides set up for the cal were left in place.
- There was no counterpart to b43_nphy_tx_cal_radio_setup_rev7(), so the
  TSSI/IQ-cal path stayed switched in. Add one, following
  wlc_phy_txcal_radio_cleanup_nphy().
- The TRSW override for the second core used 8, which does not fit the
  2-bit field on rev7+; use 2 as wl does.

With these, on a BCM43228 (N-PHY rev 16, radio 2057 rev 9) the per-step
coefficients match the proprietary wl driver's.

Found and fixed with the help of an AI coding assistant (Claude Opus 5.5),
by comparing mmiotraces of b43 and wl on the same card.

Signed-off-by: Luca Casati <luk00casati@gmail.com>
---
 drivers/net/wireless/broadcom/b43/phy_n.c | 89 +++++++++++++++++++++--
 1 file changed, 84 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/broadcom/b43/phy_n.c b/drivers/net/wireless/broadcom/b43/phy_n.c
index 66565c14e..cabf61870 100644
--- a/drivers/net/wireless/broadcom/b43/phy_n.c
+++ b/drivers/net/wireless/broadcom/b43/phy_n.c
@@ -4775,6 +4775,37 @@ static void b43_nphy_tx_cal_radio_setup_rev7(struct b43_wldev *dev)
 	}
 }
 
+/*
+ * Undo b43_nphy_tx_cal_radio_setup_rev7(), as brcmsmac's
+ * wlc_phy_txcal_radio_cleanup_nphy() does. Without it the TSSI/IQ cal
+ * path stays switched in after a TX IQ/LO cal.
+ */
+static void b43_nphy_tx_cal_radio_cleanup(struct b43_wldev *dev)
+{
+	struct b43_phy *phy = &dev->phy;
+	u16 *save = dev->phy.n->tx_rx_cal_radio_saveregs;
+	int core, off;
+	u16 r;
+
+	if (phy->rev < 7 || phy->rev >= 19)
+		return;
+
+	for (core = 0; core < 2; core++) {
+		r = core ? 0x20 : 0;
+		off = core * 11;
+
+		b43_radio_write(dev, r + R2057_TX0_TX_SSI_MASTER, save[off + 0]);
+		b43_radio_write(dev, r + R2057_TX0_IQCAL_VCM_HG, save[off + 1]);
+		b43_radio_write(dev, r + R2057_TX0_IQCAL_IDAC, save[off + 2]);
+		b43_radio_write(dev, r + R2057_TX0_TSSI_VCM, save[off + 3]);
+		b43_radio_write(dev, r + R2057_TX0_TX_SSI_MUX, save[off + 5]);
+		if (phy->radio_rev != 5)
+			b43_radio_write(dev, r + R2057_TX0_TSSIA, save[off + 6]);
+		b43_radio_write(dev, r + R2057_TX0_TSSIG, save[off + 7]);
+		b43_radio_write(dev, r + R2057_TX0_TSSI_MISC1, save[off + 8]);
+	}
+}
+
 /* https://bcm-v4.sipsolutions.net/802.11/PHY/N/TxCalRadioSetup */
 static void b43_nphy_tx_cal_radio_setup(struct b43_wldev *dev)
 {
@@ -5033,7 +5064,9 @@ static struct nphy_txgains b43_nphy_get_tx_gains(struct b43_wldev *dev)
 /* https://bcm-v4.sipsolutions.net/802.11/PHY/N/TxCalPhyCleanup */
 static void b43_nphy_tx_cal_phy_cleanup(struct b43_wldev *dev)
 {
-	u16 *regs = dev->phy.n->tx_rx_cal_phy_saveregs;
+	struct b43_phy *phy = &dev->phy;
+	struct b43_phy_n *nphy = dev->phy.n;
+	u16 *regs = nphy->tx_rx_cal_phy_saveregs;
 
 	if (dev->phy.rev >= 3) {
 		b43_phy_write(dev, B43_NPHY_AFECTL_C1, regs[0]);
@@ -5047,7 +5080,42 @@ static void b43_nphy_tx_cal_phy_cleanup(struct b43_wldev *dev)
 		b43_phy_write(dev, B43_NPHY_RFCTL_INTC2, regs[8]);
 		b43_phy_write(dev, B43_NPHY_PAPD_EN0, regs[9]);
 		b43_phy_write(dev, B43_NPHY_PAPD_EN1, regs[10]);
+
+		/*
+		 * Drop the overrides b43_nphy_tx_cal_phy_setup() set on rev7+
+		 * (as brcmsmac's wlc_phy_txcal_phycleanup_nphy()). Leaving
+		 * the 0x8 (PA power-up) override in place kills TX.
+		 */
+		if (phy->rev >= 19)
+			b43_nphy_rf_ctl_override_rev19(dev, 0x80, 0, 0, true,
+						       1);
+		else if (phy->rev >= 7)
+			b43_nphy_rf_ctl_override_rev7(dev, 0x80, 0, 0, true, 1);
+
 		b43_nphy_reset_cca(dev);
+
+		if (nphy->use_int_tx_iq_lo_cal) {
+			if (phy->rev >= 19) {
+				b43_nphy_rf_ctl_override_rev19(dev, 0x8, 0, 0x3,
+							       true, 0);
+			} else if (phy->rev >= 8) {
+				b43_nphy_rf_ctl_override_rev7(dev, 0x8, 0, 0x3,
+							      true, 0);
+			} else if (phy->rev == 7) {
+				bool ghz2 = b43_current_band(dev->wl) ==
+					    NL80211_BAND_2GHZ;
+
+				b43_radio_maskset(dev, ghz2 ?
+						  R2057_PAD2G_TUNE_PUS_CORE0 :
+						  R2057_IPA5G_CASCOFFV_PU_CORE0,
+						  ~1, 1);
+				b43_radio_maskset(dev, ghz2 ?
+						  R2057_PAD2G_TUNE_PUS_CORE1 :
+						  R2057_IPA5G_CASCOFFV_PU_CORE1,
+						  ~1, 1);
+				b43_radio_mask(dev, R2057_OVR_REG0, ~(1 << 4));
+			}
+		}
 	} else {
 		b43_phy_maskset(dev, B43_NPHY_AFECTL_C1, 0x0FFF, regs[0]);
 		b43_phy_maskset(dev, B43_NPHY_AFECTL_C2, 0x0FFF, regs[1]);
@@ -5102,7 +5170,12 @@ static void b43_nphy_tx_cal_phy_setup(struct b43_wldev *dev)
 			b43_nphy_rf_ctl_intc_override(dev, N_INTC_OVERRIDE_PA,
 						      0, 3);
 		b43_nphy_rf_ctl_intc_override(dev, N_INTC_OVERRIDE_TRSW, 2, 1);
-		b43_nphy_rf_ctl_intc_override(dev, N_INTC_OVERRIDE_TRSW, 8, 2);
+		/*
+		 * On rev7+ the TRSW field is 2 bits wide, so the legacy value
+		 * 8 for the second core ends up as 0; wl uses 2 there.
+		 */
+		b43_nphy_rf_ctl_intc_override(dev, N_INTC_OVERRIDE_TRSW,
+					      phy->rev >= 7 ? 2 : 8, 2);
 
 		regs[9] = b43_phy_read(dev, B43_NPHY_PAPD_EN0);
 		regs[10] = b43_phy_read(dev, B43_NPHY_PAPD_EN1);
@@ -5478,12 +5551,16 @@ static int b43_nphy_cal_tx_iq_lo(struct b43_wldev *dev,
 			}
 
 			b43_phy_write(dev, B43_NPHY_IQLOCAL_CMD, cmd);
+			/* bits 15:14 stay set while the cal step runs */
 			for (i = 0; i < 2000; i++) {
 				tmp = b43_phy_read(dev, B43_NPHY_IQLOCAL_CMD);
-				if (tmp & 0xC000)
+				if (!(tmp & 0xC000))
 					break;
 				udelay(10);
 			}
+			if (i == 2000)
+				b43err(dev->wl, "TX IQ/LO cal step 0x%04x timed out\n",
+				       cmd);
 
 			b43_ntab_read_bulk(dev, B43_NTAB16(15, 96), length,
 						buffer);
@@ -5500,8 +5577,9 @@ static int b43_nphy_cal_tx_iq_lo(struct b43_wldev *dev,
 		last = (dev->phy.rev < 3) ? 6 : 7;
 
 		if (!mphase || nphy->mphase_cal_phase_id == last) {
-			b43_ntab_write_bulk(dev, B43_NTAB16(15, 96), 4, buffer);
-			b43_ntab_read_bulk(dev, B43_NTAB16(15, 80), 4, buffer);
+			/* apply the result: best coeffs (96) -> active (80) */
+			b43_ntab_read_bulk(dev, B43_NTAB16(15, 96), 4, buffer);
+			b43_ntab_write_bulk(dev, B43_NTAB16(15, 80), 4, buffer);
 			if (dev->phy.rev < 3) {
 				buffer[0] = 0;
 				buffer[1] = 0;
@@ -5540,6 +5618,7 @@ static int b43_nphy_cal_tx_iq_lo(struct b43_wldev *dev,
 
 	b43_nphy_tx_cal_phy_cleanup(dev);
 	b43_ntab_write_bulk(dev, B43_NTAB16(7, 0x110), 2, save);
+	b43_nphy_tx_cal_radio_cleanup(dev);
 
 	if (dev->phy.rev < 2 && (!mphase || nphy->mphase_cal_phase_id == last))
 		b43_nphy_tx_iq_workaround(dev);
-- 
2.43.0

[PATCH wireless-next 3/5] b43: N-PHY: fix rev3+ RSSI calibration

[Luca Casati]

Three bugs in b43_nphy_rev3_rssi_cal():

- On rev7+ the selected VCM was written using the loop counter (8 after
  the search loop) instead of vcm_final; the 3-bit field turned that
  into VCM 0 on every rev7+ PHY.
- The distance used I*I + Q*I instead of I*I + Q*Q (brcmsmac
  wlc_phy_rssi_cal_nphy_rev3()).
- Negative W1/W2 offsets were rounded as -(abs(x + 4) / 8) instead of
  -((abs(x) + 4) / 8).

Found and fixed with the help of an AI coding assistant (Claude Opus 5.5),
by comparing mmiotraces of b43 and wl on the same card.

Signed-off-by: Luca Casati <luk00casati@gmail.com>
---
 drivers/net/wireless/broadcom/b43/phy_n.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/broadcom/b43/phy_n.c b/drivers/net/wireless/broadcom/b43/phy_n.c
index cabf61870..44769dbb7 100644
--- a/drivers/net/wireless/broadcom/b43/phy_n.c
+++ b/drivers/net/wireless/broadcom/b43/phy_n.c
@@ -2167,7 +2167,7 @@ static void b43_nphy_rev3_rssi_cal(struct b43_wldev *dev)
 				continue;
 			for (vcm = 0; vcm < 8; vcm++) {
 				currd = results[vcm][i] * results[vcm][i] +
-					results[vcm][i + 1] * results[vcm][i];
+					results[vcm][i + 1] * results[vcm][i + 1];
 				if (currd < mind) {
 					mind = currd;
 					minvcm = vcm;
@@ -2179,12 +2179,16 @@ static void b43_nphy_rev3_rssi_cal(struct b43_wldev *dev)
 			results_min[i] = minpoll;
 		}
 
-		/* Select the best VCM */
+		/*
+		 * Select the best VCM. (This used to write the loop counter,
+		 * 8, which the 3-bit field turned into VCM 0 on every rev7+
+		 * PHY.)
+		 */
 		if (dev->phy.rev >= 7)
 			b43_radio_maskset(dev,
 					  core ? R2057_NB_MASTER_CORE1 :
 						 R2057_NB_MASTER_CORE0,
-					  ~R2057_VCM_MASK, vcm);
+					  ~R2057_VCM_MASK, vcm_final);
 		else
 			b43_radio_maskset(dev, r | B2056_RX_RSSI_MISC,
 					  0xE3, vcm_final << 2);
@@ -2219,7 +2223,7 @@ static void b43_nphy_rev3_rssi_cal(struct b43_wldev *dev)
 				if (j / 2 == core) {
 					offset[j] = 232 - poll_results[j];
 					if (offset[j] < 0)
-						offset[j] = -(abs(offset[j] + 4) / 8);
+						offset[j] = -((abs(offset[j]) + 4) / 8);
 					else
 						offset[j] = (offset[j] + 4) / 8;
 					b43_nphy_scale_offset_rssi(dev, 0,
-- 
2.43.0

[PATCH wireless-next 4/5] b43: N-PHY: wait for the first 2057 RC calibration run

[Luca Casati]

RCCAL_DONE_OSCCAP still shows the previous run's done bit for a few us
after a start, and the first (SCAP) run polled it immediately, so the
run was stopped before it happened. RCCAL_SCAP_VAL then read 0 (wl
gets 8 on a BCM43228), which detunes the TX low-pass filter set up in
the rev7+ workarounds. Sleep before polling, as the other two runs do.

Found and fixed with the help of an AI coding assistant (Claude Opus 5.5),
by comparing mmiotraces of b43 and wl on the same card.

Signed-off-by: Luca Casati <luk00casati@gmail.com>
---
 drivers/net/wireless/broadcom/b43/phy_n.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/broadcom/b43/phy_n.c b/drivers/net/wireless/broadcom/b43/phy_n.c
index 44769dbb7..cd50614ff 100644
--- a/drivers/net/wireless/broadcom/b43/phy_n.c
+++ b/drivers/net/wireless/broadcom/b43/phy_n.c
@@ -938,8 +938,15 @@ static u16 b43_radio_2057_rccal(struct b43_wldev *dev)
 	}
 	b43_radio_write(dev, R2057_RCCAL_X1, 0x6E);
 
-	/* Start, wait, stop */
+	/*
+	 * Start, wait, stop. DONE still holds the previous run's done bit
+	 * for a few us after the start, so give the cal time to clear it
+	 * before polling, as for the other two runs. Polling right away
+	 * stopped this (SCAP) cal before it ran and left RCCAL_SCAP_VAL at
+	 * 0, which detunes the TX LPF on radio rev 9.
+	 */
 	b43_radio_write(dev, R2057_RCCAL_START_R1_Q1_P1, 0x55);
+	usleep_range(70, 140);
 	if (!b43_radio_wait_value(dev, R2057_RCCAL_DONE_OSCCAP, 2, 2, 500,
 				  5000000))
 		b43dbg(dev->wl, "Radio 0x2057 rccal timeout\n");
-- 
2.43.0

[PATCH wireless-next 5/5] b43: N-PHY: add the missing override to the rev7 TX_PU command

[Luca Casati]

brcmsmac's NPHY_REV7_RfctrlOverride_cmd_tx_pu also sets field 0x4 of
rev7 override bank 2 (REV7_RF_CTL_OVER5/6); b43 left it out. With it,
the override writes match brcmsmac and the wl driver on a BCM43228.

Found and fixed with the help of an AI coding assistant (Claude Opus 5.5),
by comparing mmiotraces of b43 and wl on the same card.

Signed-off-by: Luca Casati <luk00casati@gmail.com>
---
 drivers/net/wireless/broadcom/b43/phy_n.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/wireless/broadcom/b43/phy_n.c b/drivers/net/wireless/broadcom/b43/phy_n.c
index cd50614ff..05e8ce19b 100644
--- a/drivers/net/wireless/broadcom/b43/phy_n.c
+++ b/drivers/net/wireless/broadcom/b43/phy_n.c
@@ -220,6 +220,7 @@ static void b43_nphy_rf_ctl_override_one_to_many(struct b43_wldev *dev,
 		b43_nphy_rf_ctl_override_rev7(dev, 0x4, value, core, off, 0);
 		b43_nphy_rf_ctl_override_rev7(dev, 0x2, value, core, off, 1);
 		b43_nphy_rf_ctl_override_rev7(dev, 0x1, value, core, off, 2);
+		b43_nphy_rf_ctl_override_rev7(dev, 0x4, value, core, off, 2);
 		b43_nphy_rf_ctl_override_rev7(dev, 0x0800, 1, core, off, 1);
 		break;
 	case N_RF_CTL_OVER_CMD_RX_GAIN:
-- 
2.43.0