Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Sean Mollet <sean@malmoset.com>
Content-Type: text/plain;
        charset=utf-8
Content-Transfer-Encoding: quoted-printable
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3731.600.7\))
Subject: [PATCH] RTW88 firmware download issues - improvement, but not perfect
Message-Id: <491EE697-0DFE-433A-97EA-F7D40E9FE0A5@malmoset.com>
Date:   Sat, 15 Jul 2023 08:23:32 -0500
To:     linux-wireless@vger.kernel.org
Precedence: bulk

I=E2=80=99m attempting to debug intermittent firmware download issues =
that occur with rtw88 on an RTL8821CU. I did not have these problems =
with the Realtek OOT driver (using morrownr=E2=80=99s version).

Based on GitHub issues I=E2=80=99ve found, this same problem seems to =
also occasionally occur with other chips in the family, including PCI-E =
ones.

Example dmesg including some telemetry I added to narrow down the issue:

[   28.486954] rtx88: Loading firmware rtw88/rtw8821c_fw.bin
[   28.492907] rtw_8821cu 1-1.5:1.0: Firmware version 24.11.0, H2C =
version 12
[   28.988012] check_hw_ready failed
[   28.991624] rtx_88 failed in download_firmware_validate
[   28.998626] rtw_8821cu 1-1.5:1.0: failed to download firmware
[   29.012373] rtw_8821cu 1-1.5:1.0: failed to setup chip efuse info
[   29.018749] rtw_8821cu 1-1.5:1.0: failed to setup chip information
[   29.029496] rtw_8821cu: probe of 1-1.5:1.0 failed with error -22

It=E2=80=99s failing in mac.c, in the call to =
download_firmware_validate. The register contains 0x4078 instead of the =
0xC078 that is usually present after a download.

Comparing this to the OOT driver, I noticed that the order of operations =
at the end of the process is different. In rtw88, lte_coe_backup is =
restored before checking the register. In the OOT,  it=E2=80=99s done =
after. Another difference is that the check loop in OOT has a much =
larger count and a larger delay. I applied both of these changes to =
rtw88 and the failure rate decreased significantly. It is still non 0.

I happen to have a nearly ideal test lab for exploring this problem in =
that I=E2=80=99ve got a factory full of embedded systems with the =
RTL8821CU chip attached to an automated boot/provisioning system. I can =
make a change and deploy+test on 10s or even hundreds of devices in a =
few minutes.=20

I don=E2=80=99t have chip documentation, so I=E2=80=99m shooting in a =
dark a bit here. My suspicion is that this is a race condition either in =
the rtw88 driver or in the hardware or the interaction between the two. =
It also seems to be exacerbated by high IO on the host CPU during driver =
loading. I=E2=80=99m further exploring the differences between the OOT =
driver and rtw88=E2=80=99s handling of firmware download, since I=E2=80=99=
ve never seen this happen with the OOT driver.

References:

=
https://github.com/morrownr/8821cu-20210916/blob/main/hal/halmac/halmac_88=
xx/halmac_fw_88xx.c
Line 205 restores the lte_coe_backup. The equivalent of the check in =
download_firmware_validate happens inside the call to flfw_end_flow_88xx =
on line 201, the opposite order compared to what
Line 666 sets the check loop, equivalent to rtw88's =
download_firmware_validate function to 5000 cycles and the delay used is =
50 uS on line 678.

diff --git a/mac.c b/mac.c
index 298663b..d595711 100644
--- a/mac.c
+++ b/mac.c
@@ -794,15 +794,15 @@ static int __rtw_download_firmware(struct rtw_dev =
*rtwdev,

        wlan_cpu_enable(rtwdev, true);

-       if (!ltecoex_reg_write(rtwdev, 0x38, ltecoex_bckp)) {
-               ret =3D -EBUSY;
-               goto dlfw_fail;
-       }
-
        ret =3D download_firmware_validate(rtwdev);
        if (ret)
                goto dlfw_fail;

+       if (!ltecoex_reg_write(rtwdev, 0x38, ltecoex_bckp)) {
+               ret =3D -EBUSY;
+               goto dlfw_fail;
+       }
+
        /* reset desc and index */
        rtw_hci_setup(rtwdev);

diff --git a/util.c b/util.c
index ff3c269..fbd6599 100644
--- a/util.c
+++ b/util.c
@@ -10,11 +10,11 @@ bool check_hw_ready(struct rtw_dev *rtwdev, u32 =
addr, u32 mask, u32 target)
 {
        u32 cnt;

-       for (cnt =3D 0; cnt < 1000; cnt++) {
+       for (cnt =3D 0; cnt < 5000; cnt++) {
                if (rtw_read32_mask(rtwdev, addr, mask) =3D=3D target)
                        return true;

-               udelay(10);
+               udelay(50);
        }

        return false;
--=