Re: [PATCH] staging: rtl8723bs: fix inverted HT40 secondary channel offset

[Dan Carpenter <error27@gmail.com>]

On Sun, Jul 12, 2026 at 01:11:00PM +0900, teirua wrote:
> From: MinJea Kim <qndkdrnl@gmail.com>
> 
> rtw_get_chan_type() maps the driver's channel offset to nl80211 channel
> types the wrong way around.
> 
> In this driver HAL_PRIME_CHNL_OFFSET_LOWER means the primary channel is
> the lower 20 MHz half of the 40 MHz pair, i.e. the secondary channel is
> above the primary one: rtw_get_center_ch() computes the center channel
> as "channel + 2" for OFFSET_LOWER, and bwmode_update_check() sets
> OFFSET_LOWER when the AP's HT operation IE announces SCA (secondary
> channel above). In nl80211 terms that is NL80211_CHAN_HT40PLUS, not
> HT40MINUS.
> 
> Because of the inversion, cfg80211_rtw_get_channel() reports an HT40+
> association as HT40-. For an HT40+ AP on a low channel (e.g. channel 3)
> the resulting chandef spans below the 2.4 GHz band edge and is invalid,
> so the regulatory core tears the connection down 60 seconds
> (REG_ENFORCE_GRACE_MS) after the AP's country IE triggers a regdomain
> change: reg_check_chans_work() considers the reported chandef unusable
> and calls cfg80211_leave(). The supplicant then reconnects, the country
> IE changes the regdomain again, and the cycle repeats, causing a
> disconnect/reconnect loop every ~65 seconds for as long as the link is
> up.
> 
> Observed on a TECLAST X80 Power tablet (RTL8723BS) associated to an
> HT40+ AP on channel 3 with a KR country IE; a kprobe trace showed
> cfg80211_disconnect() being invoked from reg_check_chans_work(). With
> the mapping fixed, "iw dev wlan0 info" reports the correct
> "width: 40 MHz, center1: 2432 MHz" and the periodic disconnects stop.
> 
> Fixes: 5402cc178c5d ("staging: rtl8723bs: add get_channel cfg80211 implementation")
> Cc: stable@vger.kernel.org
> Assisted-by: Claude-Code:claude-fable-5 bpftrace
> Signed-off-by: MinJea Kim <qndkdrnl@gmail.com>
> ---
>  drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c b/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
> index 1484336..e472687 100644
> --- a/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
> +++ b/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
> @@ -1949,7 +1949,12 @@ static u8 rtw_get_chan_type(struct adapter *adapter)
>  		else
>  			return NL80211_CHAN_NO_HT;
>  	case CHANNEL_WIDTH_40:
> -		if (mlme_ext->cur_ch_offset == HAL_PRIME_CHNL_OFFSET_UPPER)
> +		/*
> +		 * HAL_PRIME_CHNL_OFFSET_LOWER means the primary channel is
> +		 * the lower 20 MHz half, i.e. the secondary channel sits
> +		 * above it (SCA), which is NL80211_CHAN_HT40PLUS.
> +		 */

AI always adds these comments...

Normally when someone fixes a bug we just allow comments like this
because if you fix a bug then you get some say in the style of the
code.  But with AI writing more and more code, maybe we should start
talking about nits like this?

The documentation for HAL_PRIME_CHNL_OFFSET_LOWER should go at the
point where it is declared.  (I haven't looked to see what is
there).  I suspect just having this explanation in the commit message
is enough, but I don't really want every constant explained every
time we use it.

regards,
dan carpenter

> +		if (mlme_ext->cur_ch_offset == HAL_PRIME_CHNL_OFFSET_LOWER)
>  			return NL80211_CHAN_HT40PLUS;
>  		else
>  			return NL80211_CHAN_HT40MINUS;
> -- 
> 2.43.0
>