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;
Subject: Re: [PATCH v5] brcmfmac: firmware: Fix firmware loading
To:     Arend van Spriel <aspriel@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>
References: <20210808180510.8753-1-digetx@gmail.com>
 <902984be-2c27-e6b0-247b-8d9a94c08cc4@broadcom.com>
 <36277b51-b647-898c-c1b4-a0e363c52b0d@gmail.com>
 <17b2b945e88.279b.9696ff82abe5fb6502268bdc3b0467d4@gmail.com>
From:   Dmitry Osipenko <digetx@gmail.com>
Message-ID: <06d7d16b-b2ca-dd67-f6a3-06b3b8b1722f@gmail.com>
Date:   Mon, 9 Aug 2021 18:58:39 +0300
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.11.0
MIME-Version: 1.0
In-Reply-To: <17b2b945e88.279b.9696ff82abe5fb6502268bdc3b0467d4@gmail.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Precedence: bulk

09.08.2021 18:42, Arend van Spriel пишет:
> 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.

Please reply with yours r-b to the patch, it should be enough.