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

[Alessio Ferri <alessio.ferri@mythread.it>]

Il giorno Mon, 26 Oct 2026 19:15:39 +0100
Luca Casati <luk00casati@gmail.com> ha scritto:

> 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);

Drop the added comment, claude like to add random and useless comments
all over the code. Changing rot and angle type is not strictly needed,
but ok for consistency with brcmsmac.

The rest of the patch is good, i encountered the same problem.

Alessio