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 v2] brcmfmac: firmware: Fix firmware loading
To:     Linus Walleij <linus.walleij@linaro.org>
Cc:     Arend van Spriel <aspriel@gmail.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>,
        linux-wireless <linux-wireless@vger.kernel.org>,
        Stefan Hansson <newbyte@disroot.org>,
        Arend van Spriel <arend.vanspriel@broadcom.com>
References: <20210804153434.3429348-1-linus.walleij@linaro.org>
 <74e24409-aeb2-5297-6888-7af2b1126ccd@gmail.com>
 <CACRpkdam=PzCf4ojxb4ASjyPBmVM8ZDWDFXqX+BukhQ+f38J0A@mail.gmail.com>
From:   Dmitry Osipenko <digetx@gmail.com>
Message-ID: <a32bc17b-a895-0e8b-0f2f-bc0abe9c7efe@gmail.com>
Date:   Thu, 5 Aug 2021 13:06:57 +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: <CACRpkdam=PzCf4ojxb4ASjyPBmVM8ZDWDFXqX+BukhQ+f38J0A@mail.gmail.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Precedence: bulk

05.08.2021 12:17, Linus Walleij пишет:
> On Thu, Aug 5, 2021 at 3:35 AM Dmitry Osipenko <digetx@gmail.com> wrote:
>> 04.08.2021 18:34, Linus Walleij пишет:
>>> +     bool tested_board_variant;
>>
>> What about s/tested/tried/?
> 
> OK that is clearer, I fix!
> 
>>> -static void brcmf_fw_request_done(const struct firmware *fw, void *ctx)
>>> +static void brcmf_fw_request_done_first(const struct firmware *fw, void *ctx)
>>
>> Is it really worthwhile to rename this function? There is no "done_second".
> 
> It is to reflect the actual use, because it fooled me as it could
> be interpreted (intuitively) as "this is called when all firmware requests
> are done" since it doesn't specify. But that is not the case, it is
> only called when done with the first first firmware in the list.
> Hence the name change.

AFAICS, it's called for both first and when done.

> The philosophy is in line with Rusty Russell's API design hierarchy:
> http://sweng.the-davies.net/Home/rustys-api-design-manifesto

Why you can't split that function in two then?

brcmf_fw_request_done_first()
{
	if (!fw) {
		request_firmware_nowait(THIS_MODULE, true, first->path,
					fwctx->dev, GFP_KERNEL, fwctx,
					brcmf_fw_request_done);
	} else {
		brcmf_fw_request_done();
	}
}