mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Christian Hewitt <christianshewitt@gmail.com>
To: Ping-Ke Shih <pkshih@realtek.com>
Cc: Bitterblue Smith <rtl8821cerfe2@gmail.com>,
	"linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] wifi: rtw89: retry efuse physical map dump on transient failure
Date: Tue, 17 Mar 2026 10:15:01 +0400	[thread overview]
Message-ID: <BE076023-7D25-4AF0-B01C-4AA69E3666CD@gmail.com> (raw)
In-Reply-To: <43c8229cbe284b25bd1fd6b3a66a753d@realtek.com>

> On 17 Mar 2026, at 5:37 am, Ping-Ke Shih <pkshih@realtek.com> wrote:
> 
> Christian Hewitt <christianshewitt@gmail.com> wrote:
>>> On 16 Mar 2026, at 9:32 am, Ping-Ke Shih <pkshih@realtek.com> wrote:
>>> 
>>> Christian Hewitt <christianshewitt@gmail.com> 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@gmail.com>
>>> 
>>> [...]
>>> 
>>>> 
>>>> 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) \
>> 
>> I’ve run some boot tests and this also resolves my efuse map use-case, e.g.
>> 
>> ROCK5B:~ # dmesg | grep rtw89
>> [    6.506375] rtw89_8852be 0002:21:00.0: loaded firmware
>> rtw89/rtw8852b_fw-1.bin
>> [    6.506539] rtw89_8852be 0002:21:00.0: enabling device (0000 -> 0003)
>> [    6.516069] rtw89_8852be 0002:21:00.0: Firmware version 0.29.29.15
>> (6fb3ec41), cmd version 0, type 5
>> [    6.516083] rtw89_8852be 0002:21:00.0: Firmware version 0.29.29.15
>> (6fb3ec41), cmd version 0, type 3
>> [   10.153731] rtw89_8852be 0002:21:00.0: efuse dump (dav=0) failed, retrying
>> (0)
>> [   10.405347] rtw89_8852be 0002:21:00.0: chip info CID: 0, CV: 1, AID: 0, ACV:
>> 1, RFE: 1
>> [   10.408311] rtw89_8852be 0002:21:00.0: rfkill hardware state changed to
>> enable
>> 
>> So far I haven’t observed more than 1x retry being required, and there are no
>> issues with loading the BT module.
> 
> My changes do retry for 5 times, because your patch does 3 times retry plus
> additional 500ms delay. I feel you want around 5 seconds for loading BT module.

Understood.

> Did you mean for now you can't reproduce the situation that long loading
> time of BT module? (But it took long time days ago?)

I’ve never noticed a long loading time for the BT module and timings seem
to be consistent both with and without the patch (and not a problem).

>> Would you like me to send a v2 using your revised version? - or?
> 
> Yes, please help v2.

Thanks, will send v2 later today.

Christian

      reply	other threads:[~2026-03-17  6:15 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-01  4:24 Christian Hewitt
2026-03-02  5:47 ` Ping-Ke Shih
2026-03-02  5:55   ` Christian Hewitt
2026-03-02  6:04     ` Ping-Ke Shih
2026-03-02  6:17       ` Christian Hewitt
2026-03-09  2:35         ` Ping-Ke Shih
2026-03-10 17:16           ` Christian Hewitt
2026-03-11  3:05             ` Ping-Ke Shih
2026-03-11  4:20               ` Christian Hewitt
2026-03-12  2:22                 ` Ping-Ke Shih
2026-03-12  5:58                   ` Christian Hewitt
2026-03-12  7:39                     ` Ping-Ke Shih
2026-03-12  8:11                       ` Christian Hewitt
2026-03-12  8:28                         ` Ping-Ke Shih
2026-03-16  5:32 ` Ping-Ke Shih
2026-03-16 11:03   ` Christian Hewitt
2026-03-17  1:37     ` Ping-Ke Shih
2026-03-17  6:15       ` Christian Hewitt [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=BE076023-7D25-4AF0-B01C-4AA69E3666CD@gmail.com \
    --to=christianshewitt@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=pkshih@realtek.com \
    --cc=rtl8821cerfe2@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®