Bitterblue Smith <rtl8821cerfe2@xxxxxxxxx> wrote:
> 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.
I guess this is because vendor driver does, and pointer is reasonable.
>
> >> + param->en = 0;
> >> +
> >> if (param_ini.pub_cfg)
> >> param->pub_cfg = *param_ini.pub_cfg;
> >>
> >
> >
>
