Re: A workaround for request_firmware() stuck in module_init

[Takashi Iwai <tiwai@suse.de>]

At Wed, 5 Sep 2012 13:59:56 -0300,
Lucas De Marchi wrote:
> 
> On Wed, Sep 5, 2012 at 10:03 AM, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
> >> If the driver is built in kernel, the request_firmware in .probe() may
> >> prolong kernel init, and it might be a problem. But looks it is not a
> >> big deal since most of drivers are built as module.
> >
> > Doing it by deferring the load also fixes that. The built in ones will
> > defer their final probe until the firmware appears and all will be well.
> >
> > If your rootfs needs firmware not in your initrd you already broke it and
> > there is a certain level beyond which you just have to give up trying to
> > save people from themselves.
> >
> > It may actually make sense to push more of it into the core driver layer
> > and take some of the ability to make mistakes away from driver authors.
> > For the general case of "load firmware if we see one" there isn't really
> > any reason we can't have a firmware_name entry in the probe table
> > entries themselves. If that was present the core bus probe would kick a
> > firmware load off and only when the firmware had loaded would it call
> > ->probe with dev->firmware pointing at a refcounted firmware struct.
> >
> > At that point it should be much faster to fix existing drivers and much
> > harder for a random device driver to get it wrong. We can even add
> > helpers which manage dev->firmware, and free the relevant objects when
> > needed, plus doing automatic ref/deref on probe/remove so that for a
> > typical driver the author only has to do
> >
> >         {PCI_blah , ... .firmware_name="wibble500.xcr", }
> >
> > and all the loading, unloading, not loading twice happens by "magic" for
> > the driver author.
> >
> > Add a dev_discard_firmware() for drivers that do this and know they can
> > then dump the file and all is good 8)
> 
> 
> It seems like a good plan. So drivers that call request_module()
> inside init_module() can be easily converted to this new scheme.
> 
> For those drivers that load the firmware upon open() syscal can be
> left as is, right?
> 
> Then we can write the rule in stone: *don't call request_firmware from
> init_module, instead give the name of the firmware*.

And we can even add a WARNING() if the call still happens, once when
the new implementation is done.


Takashi

> I even see
> drivers whose only purpose is to load the firmware and change the PID
> so it's handled by another module (like drivers/bluetooth/bcm203x.c)
> to be simplified by some extent.
> 
> 
> Lucas De Marchi
>