On 16/06/2025 04:26, Ping-Ke Shih wrote:
> Bitterblue Smith <rtl8821cerfe2@xxxxxxxxx> wrote:
>> USB and SDIO will need different sets of values, so make hfc_param_ini
>> in struct rtw89_chip_info an array.
>>
>> Signed-off-by: Bitterblue Smith <rtl8821cerfe2@xxxxxxxxx>
>
> [...]
>
>
>> diff --git a/drivers/net/wireless/realtek/rtw89/mac.c b/drivers/net/wireless/realtek/rtw89/mac.c
>> index 2ee766e37322..0a3603f2e6bf 100644
>> --- a/drivers/net/wireless/realtek/rtw89/mac.c
>> +++ b/drivers/net/wireless/realtek/rtw89/mac.c
>> @@ -881,13 +881,14 @@ static int hfc_reset_param(struct rtw89_dev *rtwdev)
>
> By the way, please help to remove the initializer of param_ini, which is weird
> and unnecessary:
>
> struct rtw89_hfc_param_ini param_ini = {NULL};
>
>>
>> switch (rtwdev->hci.type) {
>> case RTW89_HCI_TYPE_PCIE:
>> - param_ini = rtwdev->chip->hfc_param_ini[qta_mode];
>> - param->en = 0;
>> break;
>> default:
>> return -EINVAL;
>> }
>>
>> + param_ini = rtwdev->chip->hfc_param_ini[rtwdev->hci.type][qta_mode];
>
> Like patch 02/14, we can remove switch...case above, and check
> rtwdev->chip->hfc_param_ini[rtwdev->hci.type] to know if we can support
> certain HCI, like
>
> param_inis = rtwdev->chip->hfc_param_ini[rtwdev->hci.type];
> if (!param_inis)
> return -EINVAL;
>
> param_ini = param_inis[qta_mode];
>
>
That reminds me, why make a copy? param_ini could be a pointer instead.
>> + param->en = 0;
>> +
>> if (param_ini.pub_cfg)
>> param->pub_cfg = *param_ini.pub_cfg;
>>
>
>
