RE: [PATCH 03/20] wifi: rtw88: Allow different C2H RA report sizes

[Ping-Ke Shih <pkshih@realtek.com>]

Bitterblue Smith <rtl8821cerfe2@gmail.com> wrote:
> On 15/08/2024 09:14, Ping-Ke Shih wrote:
> > Bitterblue Smith <rtl8821cerfe2@gmail.com> wrote:
> >> The RTL8821AU and RTL8812AU have smaller RA report size, only 4 bytes.
> >> Avoid the "invalid ra report c2h length" error.
> >>
> >> Signed-off-by: Bitterblue Smith <rtl8821cerfe2@gmail.com>
> >> ---
> >>  drivers/net/wireless/realtek/rtw88/fw.c       | 8 ++++++--
> >>  drivers/net/wireless/realtek/rtw88/main.h     | 1 +
> >>  drivers/net/wireless/realtek/rtw88/rtw8703b.c | 1 +
> >>  drivers/net/wireless/realtek/rtw88/rtw8723d.c | 1 +
> >>  drivers/net/wireless/realtek/rtw88/rtw8821c.c | 1 +
> >>  drivers/net/wireless/realtek/rtw88/rtw8822b.c | 1 +
> >>  drivers/net/wireless/realtek/rtw88/rtw8822c.c | 1 +
> >>  7 files changed, 12 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/net/wireless/realtek/rtw88/fw.c b/drivers/net/wireless/realtek/rtw88/fw.c
> >> index 782f3776e0a0..ac53e3e30af0 100644
> >> --- a/drivers/net/wireless/realtek/rtw88/fw.c
> >> +++ b/drivers/net/wireless/realtek/rtw88/fw.c
> >> @@ -157,7 +157,10 @@ static void rtw_fw_ra_report_iter(void *data, struct ieee80211_sta *sta)
> >>
> >>         rate = GET_RA_REPORT_RATE(ra_data->payload);
> >>         sgi = GET_RA_REPORT_SGI(ra_data->payload);
> >> -       bw = GET_RA_REPORT_BW(ra_data->payload);
> >> +       if (si->rtwdev->chip->c2h_ra_report_size < 7)
> >
> > Explicitly specify '== 4' for the case of RTL8821AU and RTL8812AU.
> >
> >> +               bw = si->bw_mode;
> >> +       else
> >> +               bw = GET_RA_REPORT_BW(ra_data->payload);
> >>
> >
> >
> 
> Would that make sense? I check for less than 7 because the size
> has to be at least 7 in order to access payload[6] (GET_RA_REPORT_BW).

As you did "WARN(length < rtwdev->chip->c2h_ra_report_size)", I assume you
expect "< 7" cases is only for coming chips RTL8821AU and RTL8812AU.

Maybe explicitly specifying chips ID would be easier to understand:
        if (chip == RTL8821A || chip == RTL8812A)
               bw = si->bw_mode;
        else
               bw = GET_RA_REPORT_BW(ra_data->payload);

That's why I want "== 4". (but it seems implicitly not explicitly though.)