Re: [RFC] ath10k: silence firmware file probing warnings

[Prarit Bhargava <prarit@redhat.com>]

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@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().

P.

> Stanislaw
>