RE: [PATCH] wifi: rtw89: retry efuse physical map dump on transient failure

[Ping-Ke Shih]

Christian Hewitt <christianshewitt@xxxxxxxxx> wrote:
> On Radxa Rock 5B with a RTL8852BE combo WiFi/BT card, the efuse
> physical map dump intermittently fails with -EBUSY during probe.
> The failure occurs in rtw89_dump_physical_efuse_map_ddv() where
> read_poll_timeout_atomic() times out waiting for the B_AX_EF_RDY
> bit after 1 second.
> 
> The root cause is a timing race during boot: the WiFi driver's
> chip initialization (firmware download via PCIe) overlaps with the
> Bluetooth firmware download to the same combo chip over USB. This
> can leave the efuse controller temporarily unavailable when the
> WiFi driver attempts to read the efuse map.
> 
> Add a retry loop (up to 3 attempts with 500ms delays) around the
> physical efuse map dump in rtw89_parse_efuse_map_ax(). The firmware
> download path already retries up to 5 times, but the efuse read
> that follows has no retry logic, making it the weak link in the
> probe sequence.

I'd prefer adding a wrapper to retry 5 times without delay as bottom
changes for reference. If you want to limit retry only for
'dav == false' case, it is also fine to me.

> 
> Signed-off-by: Christian Hewitt <christianshewitt@xxxxxxxxx>

[...]

> 
>  drivers/net/wireless/realtek/rtw89/efuse.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/wireless/realtek/rtw89/efuse.c
> b/drivers/net/wireless/realtek/rtw89/efuse.c
> index a2757a88d55d..d506f04ffd6c 100644
> --- a/drivers/net/wireless/realtek/rtw89/efuse.c
> +++ b/drivers/net/wireless/realtek/rtw89/efuse.c
> @@ -270,6 +270,7 @@ int rtw89_parse_efuse_map_ax(struct rtw89_dev *rtwdev)
>         u8 *log_map = NULL;
>         u8 *dav_phy_map = NULL;
>         u8 *dav_log_map = NULL;
> +       int retry;
>         int ret;
> 
>         if (rtw89_read16(rtwdev, R_AX_SYS_WL_EFUSE_CTRL) & B_AX_AUTOLOAD_SUS)
> @@ -289,7 +290,17 @@ int rtw89_parse_efuse_map_ax(struct rtw89_dev *rtwdev)
>                 goto out_free;
>         }
> 
> -       ret = rtw89_dump_physical_efuse_map(rtwdev, phy_map, 0, phy_size,
> false);
> +       for (retry = 0; retry < 3; retry++) {
> +               if (retry) {
> +                       rtw89_warn(rtwdev, "efuse dump failed, retrying
> (%d)\n",
> +                                  retry);
> +                       fsleep(500000);
> +               }
> +               ret = rtw89_dump_physical_efuse_map(rtwdev, phy_map, 0,
> +                                                   phy_size, false);
> +               if (!ret)
> +                       break;
> +       }
>         if (ret) {
>                 rtw89_warn(rtwdev, "failed to dump efuse physical map\n");
>                 goto out_free;
> --
> 2.43.0

How about retrying 5 times without fsleep(500000)?

diff --git a/drivers/net/wireless/realtek/rtw89/efuse.c b/drivers/net/wireless/realtek/rtw89/efuse.c
index a2757a88d55d..89d4b1b865f8 100644
--- a/drivers/net/wireless/realtek/rtw89/efuse.c
+++ b/drivers/net/wireless/realtek/rtw89/efuse.c
@@ -185,8 +185,8 @@ static int rtw89_dump_physical_efuse_map_dav(struct rtw89_dev *rtwdev, u8 *map,
        return 0;
 }

-static int rtw89_dump_physical_efuse_map(struct rtw89_dev *rtwdev, u8 *map,
-                                        u32 dump_addr, u32 dump_size, bool dav)
+static int __rtw89_dump_physical_efuse_map(struct rtw89_dev *rtwdev, u8 *map,
+                                          u32 dump_addr, u32 dump_size, bool dav)
 {
        int ret;

@@ -208,6 +208,25 @@ static int rtw89_dump_physical_efuse_map(struct rtw89_dev *rtwdev, u8 *map,
        return 0;
 }

+static int rtw89_dump_physical_efuse_map(struct rtw89_dev *rtwdev, u8 *map,
+                                        u32 dump_addr, u32 dump_size, bool dav)
+{
+       int retry;
+       int ret;
+
+       for (retry = 0; retry < 5; retry++) {
+               ret = __rtw89_dump_physical_efuse_map(rtwdev, map, dump_addr,
+                                                     dump_size, dav);
+               if (!ret)
+                       return 0;
+
+               rtw89_warn(rtwdev, "efuse dump (dav=%d) failed, retrying (%d)\n",
+                          dav, retry);
+       }
+
+       return ret;
+}
+
 #define invalid_efuse_header(hdr1, hdr2) \
        ((hdr1) == 0xff || (hdr2) == 0xff)
 #define invalid_efuse_content(word_en, i) \