RE: [PATCH rtw-next v1 09/13] wifi: rtw89: Fix rtw89_mac_power_switch() for USB

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

Bitterblue Smith <rtl8821cerfe2@gmail.com> wrote:
> On 13/05/2025 06:50, Ping-Ke Shih wrote:
> > Bitterblue Smith <rtl8821cerfe2@gmail.com> wrote:
> >> Clear some bits in some registers in order to allow RTL8851BU to power
> >> on. Also tested with RTL8832BU and RTL8832CU.
> >
> > I see mac_pwr_on_pcie_8851b() and mac_pwr_on_usb_8851b() at least in
> > vendor driver. If you use the power on/off functions for USB devices,
> > the problem may disappear.
> >
> 
> I modified rtw8851b_pwr_on_func() and rtw8851b_pwr_off_func() to match
> mac_pwr_on_usb_8851b() and mac_pwr_off_usb_8851b(). It wasn't enough.

The vendor driver does different things for USB and PCI, so please take
them please.

> Without this patch the firmware upload still fails:
> 
> [ 1238.299960] rtw89_8851bu 1-2:1.2: Firmware version 0.29.41.3 (65cefb31), cmd version 0, type 5
> [ 1238.299970] rtw89_8851bu 1-2:1.2: Firmware version 0.29.41.3 (65cefb31), cmd version 0, type 3
> [ 1238.523187] Bluetooth: hci1: RTL: fw version 0x048ad230
> [ 1238.607110] Bluetooth: hci1: AOSP extensions version v1.00
> [ 1238.622986] Bluetooth: MGMT ver 1.23
> [ 1388.109364] rtw89_8851bu 1-2:1.2: [ERR]H2C path ready
> [ 1388.109751] rtw89_8851bu 1-2:1.2: [ERR]fwdl 0x1E0 = 0xc1
> [ 1388.110115] rtw89_8851bu 1-2:1.2: [ERR]fwdl 0x83F0 = 0x120090
> [ 1388.111615] rtw89_8851bu 1-2:1.2: [ERR]fw PC = 0xb890cd07
> [ 1388.111984] rtw89_8851bu 1-2:1.2: [ERR]fw PC = 0xb890cde5
> [ 1388.112358] rtw89_8851bu 1-2:1.2: [ERR]fw PC = 0xb890cd2b
> [ 1388.112740] rtw89_8851bu 1-2:1.2: [ERR]fw PC = 0xb890ef05
> [ 1388.113110] rtw89_8851bu 1-2:1.2: [ERR]fw PC = 0xb890cd2b
> [ 1388.113510] rtw89_8851bu 1-2:1.2: [ERR]fw PC = 0xb890cce3
> [ 1388.113868] rtw89_8851bu 1-2:1.2: [ERR]fw PC = 0xb890cce3
> [ 1388.114234] rtw89_8851bu 1-2:1.2: [ERR]fw PC = 0xb890cded
> [ 1388.114693] rtw89_8851bu 1-2:1.2: [ERR]fw PC = 0xb890cce3
> [ 1388.114986] rtw89_8851bu 1-2:1.2: [ERR]fw PC = 0xb890cce3
> [ 1388.115358] rtw89_8851bu 1-2:1.2: [ERR]fw PC = 0xb890cde9
> [ 1388.115735] rtw89_8851bu 1-2:1.2: [ERR]fw PC = 0xb890cce3
> [ 1388.116109] rtw89_8851bu 1-2:1.2: [ERR]fw PC = 0xb890cce3
> [ 1388.116480] rtw89_8851bu 1-2:1.2: [ERR]fw PC = 0xb89957b3
> [ 1388.116867] rtw89_8851bu 1-2:1.2: [ERR]fw PC = 0xb890cce3
> 
> (By the way, 150 seconds is a long time and this is just the first batch
> of error messages. There are at least two more like the above and each
> takes another 150 seconds to appear. Things freeze:
> 
> [ 1475.693454] INFO: task systemd-journal:284 blocked for more than 122 seconds.
> [ 1475.693750] INFO: task kded6:853 blocked for more than 122 seconds.
> [ 1598.573772] INFO: task upowerd:1021 blocked for more than 122 seconds.
> 
> I haven't figured out how to fix this long delay yet.)
> 
> >>
> >> Signed-off-by: Bitterblue Smith <rtl8821cerfe2@gmail.com>
> >> ---
> >>  drivers/net/wireless/realtek/rtw89/mac.c | 8 ++++++++
> >>  drivers/net/wireless/realtek/rtw89/reg.h | 1 +
> >>  2 files changed, 9 insertions(+)
> >>
> >> diff --git a/drivers/net/wireless/realtek/rtw89/mac.c b/drivers/net/wireless/realtek/rtw89/mac.c
> >> index 99f01fff90fe..d3cf67dfabc9 100644
> >> --- a/drivers/net/wireless/realtek/rtw89/mac.c
> >> +++ b/drivers/net/wireless/realtek/rtw89/mac.c
> >> @@ -1458,6 +1458,14 @@ static int rtw89_mac_power_switch(struct rtw89_dev *rtwdev, bool on)
> >>         int ret;
> >>         u8 val;
> >>
> >> +       if (rtwdev->hci.type == RTW89_HCI_TYPE_USB &&
> >> +           rtw89_read32_mask(rtwdev, R_AX_GPIO_MUXCFG, B_AX_BOOT_MODE)) {
> >
> > Not prefer calling a function in if-statement. Suggest to add a function.
> >
> 
> Do you mean add a variable?
> 
>         boot_mode = rtw89_read32_mask(rtwdev, R_AX_GPIO_MUXCFG, B_AX_BOOT_MODE);
> 
>         if (rtwdev->hci.type == RTW89_HCI_TYPE_USB && boot_mode) {

Yes. I prefer this. But I knew originally you would not do the IO before
checking HCI type. So, I suggested to add a function like 
(the naming is not good though)

void rtw89_mac_power_switch_hci(rtwdev, ...)
{
    if (!USB)
        return;

    val = rtw89_read32_mask(rtwdev, R_AX_GPIO_MUXCFG, B_AX_BOOT_MODE));
    if (!val)
        return;

    // do the IO you want. 
}

> 
> 
> >> +               rtw89_write32_clr(rtwdev, R_AX_SYS_PW_CTRL, B_AX_APFN_ONMAC);
> >> +               rtw89_write32_clr(rtwdev, R_AX_SYS_STATUS1, B_AX_AUTO_WLPON);
> >> +               rtw89_write32_clr(rtwdev, R_AX_GPIO_MUXCFG, B_AX_BOOT_MODE);
> >> +               rtw89_write32_clr(rtwdev, R_AX_RSV_CTRL, B_AX_R_DIS_PRST);
> >> +       }
> >> +
> >>         if (on) {
> >>                 cfg_seq = chip->pwr_on_seq;
> >>                 cfg_func = chip->ops->pwr_on_func;
> >