Could we have request_firmware_nowait with FW_OPT_NO_WARN?

Could we have request_firmware_nowait with FW_OPT_NO_WARN?

[Rafał Miłecki]

Hi,

In brcmfmac we use request_firmware_nowait and if fetching firmware
with NVRAM variables fails then we try to fallback to the platform one
(see brcmf_fw_request_code_done & brcmf_fw_request_nvram_done).

Some problem for us is that on devices with platform NVRAM we get this error:
Direct firmware load for brcm/brcmfmac43602-pcie.txt failed with error -2
(which is harmless if getting platform NVRAM succeeds). This error is
quite confusing for users. They think something went wrong, they
expect problems & they report it back to us. Obviously I don't want
ugly hacks like:
pr_info("Got platform NVRAM, ignore above error\n");

So it would be nice to have version of request_firmware_nowait with
FW_OPT_NO_WARN. If requesting firmware NVRAM fails *and* getting
platform NVRAM fails, then I could to print error on my own.
Does it make sense? Can you see a point of my request?

Do you have any suggestion for this? If and how I could proceed with
implementation?

request_firmware_nowait already has "bool uevent" argument, I don't
want it to have argument per every available option. I was thinking
about moving FW_OPT_* defines to the include/linux/firmware.h but I'm
not sure if it's OK as they depend on:
CONFIG_FW_LOADER_USER_HELPER
and
CONFIG_FW_LOADER_USER_HELPER_FALLBACK
With defines placed in firmware.h I could replace "bool uevent" with
"unsigned int opt_flags".
Does it sound like a good plan? Or do you have any better idea?

-- 
Rafał

Re: Could we have request_firmware_nowait with FW_OPT_NO_WARN?

[Johannes Berg]

On Sat, 2016-12-10 at 16:54 +0100, Rafał Miłecki wrote:
> Hi,
> 
> In brcmfmac we use request_firmware_nowait and if fetching firmware
> with NVRAM variables fails then we try to fallback to the platform
> one (see brcmf_fw_request_code_done & brcmf_fw_request_nvram_done).
> 
> Some problem for us is that on devices with platform NVRAM we get
> this error:
> Direct firmware load for brcm/brcmfmac43602-pcie.txt failed with error -2

This also happens with iwlwifi, because it requests multiple firmware
versions starting at the most recent supported one (which is often not
released at the same time).

So yeah, this would be really useful - why don't you just make a patch
with some kind of flags, whether it's FW_OPT_* or new flags?

johannes

Re: Could we have request_firmware_nowait with FW_OPT_NO_WARN?

[Rafał Miłecki]

On 12 December 2016 at 09:12, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Sat, 2016-12-10 at 16:54 +0100, Rafał Miłecki wrote:
>> In brcmfmac we use request_firmware_nowait and if fetching firmware
>> with NVRAM variables fails then we try to fallback to the platform
>> one (see brcmf_fw_request_code_done & brcmf_fw_request_nvram_done).
>>
>> Some problem for us is that on devices with platform NVRAM we get
>> this error:
>> Direct firmware load for brcm/brcmfmac43602-pcie.txt failed with error -2
>
> This also happens with iwlwifi, because it requests multiple firmware
> versions starting at the most recent supported one (which is often not
> released at the same time).

Good to know it may help others as well!


> So yeah, this would be really useful - why don't you just make a patch
> with some kind of flags, whether it's FW_OPT_* or new flags?

OK! If noone will come with any special comments/ideas soon, I'll
propose a patch for using some flags.

FWIW, meanwhile I submitted
[PATCH V2] firmware: simplify defining and handling FW_OPT_FALLBACK
https://patchwork.kernel.org/patch/9469875/

-- 
Rafał

Re: Could we have request_firmware_nowait with FW_OPT_NO_WARN?

[Arend Van Spriel]

On 12-12-2016 9:32, Rafał Miłecki wrote:
> On 12 December 2016 at 09:12, Johannes Berg <johannes@sipsolutions.net> wrote:
>> On Sat, 2016-12-10 at 16:54 +0100, Rafał Miłecki wrote:
>>> In brcmfmac we use request_firmware_nowait and if fetching firmware
>>> with NVRAM variables fails then we try to fallback to the platform
>>> one (see brcmf_fw_request_code_done & brcmf_fw_request_nvram_done).
>>>
>>> Some problem for us is that on devices with platform NVRAM we get
>>> this error:
>>> Direct firmware load for brcm/brcmfmac43602-pcie.txt failed with error -2
>>
>> This also happens with iwlwifi, because it requests multiple firmware
>> versions starting at the most recent supported one (which is often not
>> released at the same time).
> 
> Good to know it may help others as well!
> 
> 
>> So yeah, this would be really useful - why don't you just make a patch
>> with some kind of flags, whether it's FW_OPT_* or new flags?
> 
> OK! If noone will come with any special comments/ideas soon, I'll
> propose a patch for using some flags.
> 
> FWIW, meanwhile I submitted
> [PATCH V2] firmware: simplify defining and handling FW_OPT_FALLBACK
> https://patchwork.kernel.org/patch/9469875/

Similar thread couple of months ago [1]

Regards,
Arend

[1] http://lists.infradead.org/pipermail/ath10k/2016-July/thread.html#8026

Re: Could we have request_firmware_nowait with FW_OPT_NO_WARN?

[Rafał Miłecki]

On 12 December 2016 at 10:26, Arend Van Spriel
<arend.vanspriel@broadcom.com> wrote:
> On 12-12-2016 9:32, Rafał Miłecki wrote:
>> On 12 December 2016 at 09:12, Johannes Berg <johannes@sipsolutions.net> wrote:
>>> On Sat, 2016-12-10 at 16:54 +0100, Rafał Miłecki wrote:
>>>> In brcmfmac we use request_firmware_nowait and if fetching firmware
>>>> with NVRAM variables fails then we try to fallback to the platform
>>>> one (see brcmf_fw_request_code_done & brcmf_fw_request_nvram_done).
>>>>
>>>> Some problem for us is that on devices with platform NVRAM we get
>>>> this error:
>>>> Direct firmware load for brcm/brcmfmac43602-pcie.txt failed with error -2
>>>
>>> This also happens with iwlwifi, because it requests multiple firmware
>>> versions starting at the most recent supported one (which is often not
>>> released at the same time).
>>
>> Good to know it may help others as well!
>>
>>
>>> So yeah, this would be really useful - why don't you just make a patch
>>> with some kind of flags, whether it's FW_OPT_* or new flags?
>>
>> OK! If noone will come with any special comments/ideas soon, I'll
>> propose a patch for using some flags.
>>
>> FWIW, meanwhile I submitted
>> [PATCH V2] firmware: simplify defining and handling FW_OPT_FALLBACK
>> https://patchwork.kernel.org/patch/9469875/
>
> Similar thread couple of months ago [1]
>
> (...)
>
> [1] http://lists.infradead.org/pipermail/ath10k/2016-July/thread.html#8026

Oh, now I see it's a bit messy topic and not clearly maintained class.
It seems more ppl were confused by the API. I think having many
unrelated behavior bounded to few functions caused some of this
confusion. Let's hope adding some flags will let us use function the
way they were designed, I'll definitely try working on this.

-- 
Rafał

Re: Could we have request_firmware_nowait with FW_OPT_NO_WARN?

[Kalle Valo]

Rafał Miłecki <zajec5@gmail.com> writes:

> On 12 December 2016 at 09:12, Johannes Berg <johannes@sipsolutions.net> wrote:
>> On Sat, 2016-12-10 at 16:54 +0100, Rafał Miłecki wrote:
>>> In brcmfmac we use request_firmware_nowait and if fetching firmware
>>> with NVRAM variables fails then we try to fallback to the platform
>>> one (see brcmf_fw_request_code_done & brcmf_fw_request_nvram_done).
>>>
>>> Some problem for us is that on devices with platform NVRAM we get
>>> this error:
>>> Direct firmware load for brcm/brcmfmac43602-pcie.txt failed with error -2
>>
>> This also happens with iwlwifi, because it requests multiple firmware
>> versions starting at the most recent supported one (which is often not
>> released at the same time).
>
> Good to know it may help others as well!

We have the same problem also on ath10k :) And it's confusing users a
lot, especially as we also load calibration file and other files. So
yes, something like this is very much needed.

But in ath10k we use request_firmware() instead
request_firmware_nowait(). So I would appreciate if you could add the
support to both variants.

-- 
Kalle Valo

Re: Could we have request_firmware_nowait with FW_OPT_NO_WARN?

[Luis R. Rodriguez]

On Mon, Dec 12, 2016 at 10:53:38AM +0100, Rafał Miłecki wrote:
> On 12 December 2016 at 10:26, Arend Van Spriel
> <arend.vanspriel@broadcom.com> wrote:
> > On 12-12-2016 9:32, Rafał Miłecki wrote:
> >> On 12 December 2016 at 09:12, Johannes Berg <johannes@sipsolutions.net> wrote:
> >>> On Sat, 2016-12-10 at 16:54 +0100, Rafał Miłecki wrote:
> >>>> In brcmfmac we use request_firmware_nowait and if fetching firmware
> >>>> with NVRAM variables fails then we try to fallback to the platform
> >>>> one (see brcmf_fw_request_code_done & brcmf_fw_request_nvram_done).
> >>>>
> >>>> Some problem for us is that on devices with platform NVRAM we get
> >>>> this error:
> >>>> Direct firmware load for brcm/brcmfmac43602-pcie.txt failed with error -2
> >>>
> >>> This also happens with iwlwifi, because it requests multiple firmware
> >>> versions starting at the most recent supported one (which is often not
> >>> released at the same time).
> >>
> >> Good to know it may help others as well!
> >>
> >>
> >>> So yeah, this would be really useful - why don't you just make a patch
> >>> with some kind of flags, whether it's FW_OPT_* or new flags?
> >>
> >> OK! If noone will come with any special comments/ideas soon, I'll
> >> propose a patch for using some flags.
> >>
> >> FWIW, meanwhile I submitted
> >> [PATCH V2] firmware: simplify defining and handling FW_OPT_FALLBACK
> >> https://patchwork.kernel.org/patch/9469875/
> >
> > Similar thread couple of months ago [1]
> >
> > (...)
> >
> > [1] http://lists.infradead.org/pipermail/ath10k/2016-July/thread.html#8026
> 
> Oh, now I see it's a bit messy topic and not clearly maintained class.
> It seems more ppl were confused by the API. I think having many
> unrelated behavior bounded to few functions caused some of this
> confusion. Let's hope adding some flags will let us use function the
> way they were designed, I'll definitely try working on this.

4.9 was just released, this means the merge window opened and no functional
changes will be merged for a while. I'll revamp my new API whcih would allow
what you describe to be an add-on flag without having to extend the API with
yet another series of exported symbols just for a new option. I'll also
CC you on a documentation revamp because as you note its much needed.

  Luis

Re: Could we have request_firmware_nowait with FW_OPT_NO_WARN?

[Luis R. Rodriguez]

On Sat, Dec 10, 2016 at 04:54:41PM +0100, Rafał Miłecki wrote:
> So it would be nice to have version of request_firmware_nowait with
> FW_OPT_NO_WARN. If requesting firmware NVRAM fails *and* getting
> platform NVRAM fails, then I could to print error on my own.
> Does it make sense? Can you see a point of my request?

request_firmware_direct() does hat you describe but this is only 
available for synchronous requests. My old sysdata patches -- which
I need to "rebrand" as the only issue found was the naming -- added
an equivalent to request_firmware_direct() for async calls. The newer
API simply has 2 API exported calls, the idea is we'd enable the caller
to configure the request as per their requirements instead of adding
a new exported routine per new feature.

I'll update docs, rebase my patches by rebranding them, and also add
a bit more as per some recent discussion to resolve the pivot_root()
races upon init if you do not use initramfs. I'll be sure to Cc you.

  Luis

Re: Could we have request_firmware_nowait with FW_OPT_NO_WARN?

[Rafał Miłecki]

On 12 December 2016 at 15:07, Luis R. Rodriguez <mcgrof@kernel.org> wrote:
> On Mon, Dec 12, 2016 at 10:53:38AM +0100, Rafał Miłecki wrote:
>> On 12 December 2016 at 10:26, Arend Van Spriel
>> <arend.vanspriel@broadcom.com> wrote:
>> > On 12-12-2016 9:32, Rafał Miłecki wrote:
>> >> On 12 December 2016 at 09:12, Johannes Berg <johannes@sipsolutions.net> wrote:
>> >>> On Sat, 2016-12-10 at 16:54 +0100, Rafał Miłecki wrote:
>> >>>> In brcmfmac we use request_firmware_nowait and if fetching firmware
>> >>>> with NVRAM variables fails then we try to fallback to the platform
>> >>>> one (see brcmf_fw_request_code_done & brcmf_fw_request_nvram_done).
>> >>>>
>> >>>> Some problem for us is that on devices with platform NVRAM we get
>> >>>> this error:
>> >>>> Direct firmware load for brcm/brcmfmac43602-pcie.txt failed with error -2
>> >>>
>> >>> This also happens with iwlwifi, because it requests multiple firmware
>> >>> versions starting at the most recent supported one (which is often not
>> >>> released at the same time).
>> >>
>> >> Good to know it may help others as well!
>> >>
>> >>
>> >>> So yeah, this would be really useful - why don't you just make a patch
>> >>> with some kind of flags, whether it's FW_OPT_* or new flags?
>> >>
>> >> OK! If noone will come with any special comments/ideas soon, I'll
>> >> propose a patch for using some flags.
>> >>
>> >> FWIW, meanwhile I submitted
>> >> [PATCH V2] firmware: simplify defining and handling FW_OPT_FALLBACK
>> >> https://patchwork.kernel.org/patch/9469875/
>> >
>> > Similar thread couple of months ago [1]
>> >
>> > (...)
>> >
>> > [1] http://lists.infradead.org/pipermail/ath10k/2016-July/thread.html#8026
>>
>> Oh, now I see it's a bit messy topic and not clearly maintained class.
>> It seems more ppl were confused by the API. I think having many
>> unrelated behavior bounded to few functions caused some of this
>> confusion. Let's hope adding some flags will let us use function the
>> way they were designed, I'll definitely try working on this.
>
> 4.9 was just released, this means the merge window opened and no functional
> changes will be merged for a while. I'll revamp my new API whcih would allow
> what you describe to be an add-on flag without having to extend the API with
> yet another series of exported symbols just for a new option. I'll also
> CC you on a documentation revamp because as you note its much needed.

I started working on request_firmware_async with "unsigned int
opt_flags" argument and making request_firmware_nowait an inline
function just passing proper flags.

Well, I guess it's always disappointing having to drop your work, but
let it be... Please Cc me with your patches as well as documentation
update.

-- 
Rafał