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

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

Hi Jie Feng,

Thanks for the new version. A few comments below.

October 31, 2026 at 8:09 AM, "Ho Jie Feng" <hjf3108@gmail.com> wrote:

> diff --git a/drivers/net/wireless/realtek/rtw88/rtw8814a.c b/drivers/net/wireless/realtek/rtw88/rtw8814a.c
> index 5bb55606f14a126806acb354658c150150ae0cce..2db05a83766d65123edc95f50a40d3f93bfb1dba 100644
> --- a/drivers/net/wireless/realtek/rtw88/rtw8814a.c
> +++ b/drivers/net/wireless/realtek/rtw88/rtw8814a.c

[ ... ]

> +static void rtw8814a_bf_enable_bfee_su(struct rtw_dev *rtwdev,
> +                                    struct rtw_vif *vif,
> +                                    struct rtw_bfee *bfee)
> +{
> +     rtw_bf_enable_bfee_su(rtwdev, vif, bfee);
> +
> +     rtw_write16(rtwdev, REG_SND_PTCL_CTRL + 1, 0x0202);

This is a 16 bit write to 0x719, which is not 2 byte aligned. On
RTL8814AE it goes through rtw_pci_write16(), a plain writew() on the
mapped BAR. As far as I know an unaligned access to device memory raises
an alignment fault on arm64, so an RTL8814AE there could oops when it
associates to an AP that is an SU beamformer.

The vendor phydm code does a 32 bit read modify write of 0x718 for these
two bytes:

        odm_set_mac_reg(p_dm, REG_SND_PTCL_CTRL_8814A,
                        MASKBYTE1 | MASKBYTE2, 0x0202);

so maybe the same would work here:

        rtw_write32_mask(rtwdev, REG_SND_PTCL_CTRL, MASKBYTE1 | MASKBYTE2,
                         0x0202);

[ ... ]

> diff --git a/drivers/net/wireless/realtek/rtw88/rtw88xxa.c b/drivers/net/wireless/realtek/rtw88/rtw88xxa.c
> index c298fa0fb2a6b94118527a297affa5e6e8ab633d..87253fffe78cd6f5b58a325eef42b58b968e2d46 100644
> --- a/drivers/net/wireless/realtek/rtw88/rtw88xxa.c
> +++ b/drivers/net/wireless/realtek/rtw88/rtw88xxa.c
> @@ -138,6 +138,7 @@ static void rtw88xxa_read_usb_type(struct rtw_dev *rtwdev)
>               efuse->hw_cap.nss = 1;
>       else
>               efuse->hw_cap.nss = 2;
> +     rtwdev->hal.bfee_sts_cap = 1;

The function already has a hal pointer, so hal->bfee_sts_cap would match
the rest of it.

[ ... ]

> +     addr_bfer_info = slot ? REG_ASSOCIATED_BFMER1_INFO :
> +                                   REG_ASSOCIATED_BFMER0_INFO;

Nit, the second line isn't aligned with REG_ASSOCIATED_BFMER1_INFO.

[ ... ]

> +     rtw_write8(rtwdev, REG_SND_PTCL_CTRL,
> +                RTW_SND_CTRL_SOUNDING_88XXA);

This fits on one line.

Best regards,
Luka Gejak