[RFC] ath10k: silence firmware file probing warnings

Arend Van Spriel arend.vanspriel at broadcom.com
Fri Jul 22 01:38:24 PDT 2016

+ Luis

On 21-7-2016 13:51, Stanislaw Gruszka wrote:
> (cc: firmware and brcmfmac maintainers)
> On Thu, Jul 21, 2016 at 06:23:11AM -0400, Prarit Bhargava wrote:
>> On 07/21/2016 04:05 AM, Stanislaw Gruszka wrote:
>>> On Thu, Jul 21, 2016 at 10:36:42AM +0300, Emmanuel Grumbach wrote:
>>>> On Thu, Jul 21, 2016 at 10:09 AM, Stanislaw Gruszka <sgruszka at redhat.com> wrote:
>>>>> On Tue, Jul 19, 2016 at 03:00:37PM +0200, Michal Kazior wrote:
>>>>>> Firmware files are versioned to prevent older
>>>>>> driver instances to load unsupported firmware
>>>>>> blobs. This is reflected with a fallback logic
>>>>>> which attempts to load several firmware files.
>>>>>> This however produced a lot of unnecessary
>>>>>> warnings sometimes confusing users and leading
>>>>>> them to rename firmware files making things even
>>>>>> more confusing.
>>>>> This happens on kernels configured with
>>>>> CONFIG_FW_LOADER_USER_HELPER_FALLBACK and cause not only ugly warnings,
>>>>> but also 60 seconds delay before loading next firmware version.
>>>>> For some reason RHEL kernel needs above config option, so this
>>>>> patch is very welcome from my perspective.
>>>> Sorry for my ignorance but how does the firmware loading work if not
>>>> with udev's help?
>>> I'm not sure exactly, but I think kernel VFS layer is capable to copy
>>> file data directly from mounted filesystem without user space helper.
>> Here's the situation: request_firmware() waits 60 seconds for udev to do its
>> loading magic via a "usermode helper".  This delay is there to allow, for
>> example, userspace to unpack or download a new firmware image or verify the
>> firmware image *in userspace* before providing it to the driver to apply to the HW.
>> Why 60 seconds?  It is arbitrary and there is no way for udev & the kernel to
>> handshake on completion.
>>>> As you can imagine, iwlwifi is suffering from the
>>>> same problem and I would be interested in applying the same change,
>>>> but I'd love to understand a bit more :)
>>> Yes, iwlwifi (and some other drivers) suffer from this. However this
>>> happen when the newest firmware version is not installed on the system
>>> and CONFIG_FW_LOADER_USER_HELPER_FALLBACK is enabled. What I suppose
>>> it's not common.
>> request_firmware_direct() was introduced at my request because (as you've
>> noticed) when CONFIG_FW_LOADER_USER_HELPER_FALLBACK=y drivers may stall for long
>> periods of time when starting.  The bug that this introduced was a 60 second
>> delay per logical cpu when starting a system.  On a 64 cpu system that meant the
>> boot would complete in a little over one hour.
>>> I started to see this currently, because that option was enabled on 
>>> RHEL kernel. BTW: I think Prarit iwlwifi thermal_zone problem was
>>> happened because of that, i.e. thermal device was not functional
>>> because f/w wasn't loaded due to big delay.
>>> I'm not sure if replacing to request_firmware_direct() is a good
>>> fix though. For example I can see this problem also on brcmfmac, which
>>> use request_firmware_nowait(). I think I would rather prefer special
>>> helper for firmware drivers that needs user helper and have
>>> request_firmware() be direct as default.
>> The difference between request_firmware_direct() and request_firmware() is that
>> the _direct() version does not wait the 60 seconds for udev interaction.  The
>> only userspace check performed is to see if the file is there, and if the file
>> does exist it is provided to the driver to be applied to the hardware.
>> So the real question to ask here is whether or not the ath10k, brcmfmac, and
>> iwlwifi require udev to do anything beyond checking for the existence and
>> loading the firmware image.  If they don't, then it is better to use
>> request_firmware_direct().
> They don't need that, like 99% of the drivers I think, hence changing the
> default seems to be more reasonable. However changing 3 drivers would work
> for me as well, and that change do not introduce risk of broking drivers
> that require udev fw download.
> iwlwifi and ath10k are trivial, bcrmfmac is a bit more complex as it
> use request_firmware_nowait(), so it first need to be converted to
> ordinary request_firmware(), but this should be doable and I can do
> that.

I am going bonkers here. This is the Nth time a discussion pops up on
firmware API usage. I stopped counting N :-( So the first issue was that
the INIT was taking to long as we were requesting firmware during probe
which was executed in the INIT context. So we added a worker and
register the driver from there. There was probably a reason for
switching to _no_wait() as well, but I do not recall the details. The
things is I don't know if I need user-space or not. I just need firmware
to get the device up and running. We have changed our driver a couple of
times now to accommodate something that in my opinion should have been
abstracted behind the firmware API in the first place and now here is
another proposal to change the drivers. Come on!

> However I wonder if changing that will not broke the case when
> driver is build-in in the kernel and f/w is not yet available when
> driver start to initialize. Or maybe nowadays this is not the case
> any longer, i.e. the MODULE_FIRMWARE macros assure proper f/w 
> images are build-in in the kernel or copied to initramfs?

That is a nice idea, but I have not seen any change in that area. Could
have missed it.


> Thanks
> Stanislaw

More information about the ath10k mailing list