Re: [PATCH rtw-next v3 2/5] wifi: rtw88: 88xxa: receive VHT SU beamformed transmissions

[Luka Gejak <luka.gejak@linux.dev>]

Hi Jie Feng,

Thanks for the new version. A question and a comment below.

On November 12, 2026 6:32:41 PM GMT+01:00, Ho Jie Feng <hjf3108@gmail.com>
wrote:

> diff --git a/drivers/net/wireless/realtek/rtw88/main.c b/drivers/net/wireless/realtek/rtw88/main.c
> index e243738809..fe93c93b1a 100644
> --- a/drivers/net/wireless/realtek/rtw88/main.c
> +++ b/drivers/net/wireless/realtek/rtw88/main.c

[ ... ]

> @@ -1621,10 +1622,13 @@ static void rtw_init_vht_cap(struct rtw_dev *rtwdev,
>                      0;
>       if (rtwdev->hal.rf_path_num > 1)
>               vht_cap->cap |= IEEE80211_VHT_CAP_TXSTBC;
> -     vht_cap->cap |= IEEE80211_VHT_CAP_MU_BEAMFORMEE_CAPABLE |
> -                     IEEE80211_VHT_CAP_SU_BEAMFORMEE_CAPABLE;
> -     vht_cap->cap |= (rtwdev->hal.bfee_sts_cap <<
> -                     IEEE80211_VHT_CAP_BEAMFORMEE_STS_SHIFT);
> +     if (rtw_bf_support && chip->bfer_su_max_num) {
> +             vht_cap->cap |= IEEE80211_VHT_CAP_SU_BEAMFORMEE_CAPABLE;
> +             vht_cap->cap |= rtwdev->hal.bfee_sts_cap <<
> +                             IEEE80211_VHT_CAP_BEAMFORMEE_STS_SHIFT;
> +     }
> +     if (rtw_bf_support && chip->bfer_mu_max_num)
> +             vht_cap->cap |= IEEE80211_VHT_CAP_MU_BEAMFORMEE_CAPABLE;

In v2 this was its own patch, 3/7 "correctly expose beamformee
capabilities", after Bitterblue asked on v1 for it to be split out. Was
folding it back in here intended? I couldn't find it in the v3 changelog.

I can see a reason for it. Without this part, an AP that is also a MU
beamformer makes rtw_bf_assoc() take the MU branch and return at
bfer_mu_max_num, so the new config_bfee code would never run for it. If
that is why, maybe mention it in the changelog, and in the commit message
too, since this is core code and the support_bf check now hides the
beamformee caps on every chip when beamforming is turned off.

[ ... ]

> diff --git a/drivers/net/wireless/realtek/rtw88/rtw8812a.c b/drivers/net/wireless/realtek/rtw88/rtw8812a.c
> index 55f1d3d324..5d58dff4fc 100644
> --- a/drivers/net/wireless/realtek/rtw88/rtw8812a.c
> +++ b/drivers/net/wireless/realtek/rtw88/rtw8812a.c
> @@ -931,7 +931,7 @@ static const struct rtw_chip_ops rtw8812a_ops = {
>       .phy_calibration        = rtw8812a_phy_calibration,
>       .cck_pd_set             = rtw_phy_cck_pd_set,
>       .pwr_track              = rtw8812a_pwr_track,
> -     .config_bfee            = NULL,
> +     .config_bfee            = rtw88xxa_bf_config_bfee,
>       .set_gid_table          = NULL,
>       .cfg_csi_rate           = NULL,

As far as I can see, 8812a, 8821a and 8814a are now the first chips that
can become a SU beamformee without a cfg_csi_rate op. Once rtw_bf_assoc()
has set bfee.role, the watchdog goes through

        rtw_vif_watch_dog_iter()
          rtw_dynamic_csi_rate()

and there new_csi_rate_idx is only written by that op:

        u8 new_csi_rate_idx;
        ...
        rtw_chip_cfg_csi_rate(rtwdev, rtwdev->dm_info.min_rssi,
                              bf_info->cur_csi_rpt_rate,
                              fix_rate_enable, &new_csi_rate_idx);

        if (new_csi_rate_idx != bf_info->cur_csi_rpt_rate)
                bf_info->cur_csi_rpt_rate = new_csi_rate_idx;

So if I read it right, it compares and stores an uninitialized value
there. cur_csi_rpt_rate has no other user on these chips, so it seems
harmless for now, but I think KMSAN would report it. Maybe
rtw_dynamic_csi_rate() could return early when the chip has no
cfg_csi_rate op?

Best regards,
Luka Gejak