Re: [PATCH v2] brcmfmac: firmware: Fix firmware loading

[Dmitry Osipenko <digetx@gmail.com>]

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();
	}
}