Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Arend van Spriel <aspriel@gmail.com>
To:     Dmitry Osipenko <digetx@gmail.com>,
        Arend van Spriel <arend.vanspriel@broadcom.com>,
        Franky Lin <franky.lin@broadcom.com>,
        Hante Meuleman <hante.meuleman@broadcom.com>,
        "Chi-hsien Lin" <chi-hsien.lin@infineon.com>,
        Wright Feng <wright.feng@infineon.com>,
        "Chung-hsien Hsu" <chung-hsien.hsu@infineon.com>,
        Kalle Valo <kvalo@codeaurora.org>,
        Linus Walleij <linus.walleij@linaro.org>
CC:     <linux-wireless@vger.kernel.org>,
        Stefan Hansson <newbyte@disroot.org>
Date:   Mon, 09 Aug 2021 17:42:29 +0200
Message-ID: <17b2b945e88.279b.9696ff82abe5fb6502268bdc3b0467d4@gmail.com>
In-Reply-To: <36277b51-b647-898c-c1b4-a0e363c52b0d@gmail.com>
References: <20210808180510.8753-1-digetx@gmail.com>
 <902984be-2c27-e6b0-247b-8d9a94c08cc4@broadcom.com>
 <36277b51-b647-898c-c1b4-a0e363c52b0d@gmail.com>
User-Agent: AquaMail/1.30.1-1828 (build: 103001000)
Subject: Re: [PATCH v5] brcmfmac: firmware: Fix firmware loading
MIME-Version: 1.0
Content-Type: text/plain; format=flowed; charset="UTF-8"
Content-Transfer-Encoding: 8bit
Precedence: bulk

On August 9, 2021 4:56:46 PM Dmitry Osipenko <digetx@gmail.com> wrote:

> 09.08.2021 11:23, Arend van Spriel пишет:
>> On 8/8/2021 8:05 PM, Dmitry Osipenko wrote:
>>> From: Linus Walleij <linus.walleij@linaro.org>
>>>
>>> The patch that would first try the board-specific firmware
>>> had a bug because the fallback would not be called: the
>>> asynchronous interface is used meaning request_firmware_nowait()
>>> returns 0 immediately.
>>>
>>> Harden the firmware loading like this:
>>>
>>> - If we cannot build an alt_path (like if no board_type is
>>>   specified) just request the first firmware without any
>>>   suffix, like in the past.
>>>
>>> - If the lookup of a board specific firmware fails, we get
>>>   a NULL fw in the async callback, so just try again without
>>>   the alt_path from a dedicated brcm_fw_request_done_alt_path
>>>   callback.
>>>
>>> - Drop the unnecessary prototype of brcm_fw_request_done.
>>>
>>> - Added MODULE_FIRMWARE match for per-board SDIO bins, making
>>>   userspace tools to pull all the relevant firmware files.
>> The original idea was to setup the path names in
>> brcmf_fw_alloc_request() function, but with the introduction of the
>> board_type for NVRAM files that was abandoned and we cook up alternative
>> paths. Now similar is done for the firmware files. So I would want to
>> rework the code, but for now I am going with Linus's/Your fix for the
>> sake of having the regression more or less quickly resolved.
>
> Thanks, indeed it could be improved further later on.
>
>> You reported an issue earlier where the firmware callback was called
>> from the probe context causing it to hang and it is not clear to me
>> whether that is fixed with this version of the patch.
>
> It's independent minor problem that can't be easily reproduced in
> practice and seems it existed for a long time. It's not fixed by this patch.

Ok. I understand the issue and I have a fix lined up for it. I noticed my 
reviewed-by tag got lost between V2 and latest version. Feel free to add it 
back.

Regards,
Arend