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

[Jie Feng <hjf3108@gmail.com>]

On Fri, 13 Nov 2026 at 16:09, Luka Gejak <luka.gejak@linux.dev> wrote:
>
> Hi Jie Feng,
>
> Two more small comments that I missed in my first reply.
>
> 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/rtw8814a.c b/drivers/net/wireless/realtek/rtw88/rtw8814a.c
> > index e35c57c9d1..c2a18f6be8 100644
> > --- a/drivers/net/wireless/realtek/rtw88/rtw8814a.c
> > +++ b/drivers/net/wireless/realtek/rtw88/rtw8814a.c
>
> [ ... ]
>
> > +     rtw_bf_enable_bfee_su(rtwdev, vif, bfee);
> > +
> > +     rtw_write32_mask(rtwdev, REG_SND_PTCL_CTRL, MASKBYTE1 | MASKBYTE2,
> > +                      0x0202);
>
> The other values in this function have defines, but 0x0202 is bare. The
> vendor code doesn't name it either, so maybe just a short comment on
> what these two bytes do, if you know it.
Yeah, indeed the vendor code is full of magic numbers, I don't really
know what they mean beyond guessing.

> [ ... ]
>
> > diff --git a/drivers/net/wireless/realtek/rtw88/rtw88xxa.c b/drivers/net/wireless/realtek/rtw88/rtw88xxa.c
> > index 5ffa29d181..128a18ee33 100644
> > --- a/drivers/net/wireless/realtek/rtw88/rtw88xxa.c
> > +++ b/drivers/net/wireless/realtek/rtw88/rtw88xxa.c
>
> [ ... ]
>
> > +     if (rtwdev->efuse.hw_cap.nss == 2) {
> > +             rtw_write32(rtwdev, REG_CSI_CONTENT_VALUE, 0);
> > +             csi_param = 0x01090109;
> > +     } else {
> > +             rtw_write32(rtwdev, REG_CSI_CONTENT_VALUE, 0x01081008);
> > +             csi_param = 0x01080108;
> > +     }
>
> [ ... ]
>
> > +     for (i = 0; i < ARRAY_SIZE(csi_rpt_regs); i++)
> > +             rtw_write32(rtwdev, csi_rpt_regs[i], csi_param);
>
> These numbers are hard to follow. As far as I can see csi_param is the
> same 16 bit value twice, BFMER0 in the low half and BFMER1 in the high
> half, with the layout rtw_bf_enable_bfee_su() builds from named fields:
>
>         csi_param = (u16)((coefficientsize << 10) |
>                           (codebookinfo << 8) |
>                           (grouping << 6) |
>                           (nr_index << 3) |
>                           nc_index);
>
> So here codebookinfo and nr_index are 1, and nc_index is nss minus 1.
> Maybe build it the same way, or add a comment. For REG_CSI_CONTENT_VALUE
> the vendor code has "nc = 2" and "nc = 1" comments next to the two
> values, which would already help.
I opted to avoid trying to add more meaning to the vendor code, because
I couldn't be sure if they were correct. Rather than risk incorrect
documentation,
I just put the magic numbers in, since they are used as is without modification.

I can dig the vendor code for some confirmation that the csi param format
is the same for the later chips as 88xxa.

> Best regards,
> Luka Gejak