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

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

On November 13, 2026 9:01:47 AM GMT+01:00, Jie Feng <hjf3108@gmail.com>
wrote:
>On Fri, 13 Nov 2026 at 15:46, Luka Gejak <luka.gejak@linux.dev> wrote:
>>
>> 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.
>Sorry, looks like I used the wrong commit on my local tree, will resend as
>a v4.
>Please ignore this v3.
>

Can you address all comments I have send, as they will most likely apply
to v4 as well?

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