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

[Bitterblue Smith <rtl8821cerfe2@xxxxxxxxx>]

On 20/08/2024 04:10, Ping-Ke Shih wrote:
> Bitterblue Smith <rtl8821cerfe2@xxxxxxxxx> wrote:
>> On 15/08/2024 09:14, Ping-Ke Shih wrote:
>>> Bitterblue Smith <rtl8821cerfe2@xxxxxxxxx> 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@xxxxxxxxx>
>>>> ---
>>>>  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.)
> 

I just checked, the RA report size of RTL8814AU is 6.