Message-ID: <CACVXFVO+OgvLoFMcDD0gcQnbzUW=SyR=wiSMyT-FtpjWW5sWdw@mail.gmail.com>
Date:	Wed, 5 Sep 2012 23:22:55 +0800
From:	Ming Lei <ming.lei@...onical.com>
To:	Alan Cox <alan@...rguk.ukuu.org.uk>
Cc:	Takashi Iwai <tiwai@...e.de>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	"Rafael J. Wysocki" <rjw@...k.pl>, Kay Sievers <kay@...y.org>,
	linux-kernel@...r.kernel.org,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Benjamin Herrenschmidt <benh@...nel.crashing.org>
Subject: Re: A workaround for request_firmware() stuck in module_init

On Wed, Sep 5, 2012 at 9:03 PM, Alan Cox <alan@...rguk.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.

Yes, deferring the load may fix the built in case, but which also
introduces much work on changes of current drivers. In fact,
there are few guys who complained the built in case.

The current complaint is from that udev may timeout request inside probe()
when drivers are built as module. As pointed by Linus and Benjamin,
it is better to fix it in udev, and looks not good to introduce great changes
(such as Takashi's defer probe patch) in kernel to workaround the problem.

>
> 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

Linus has said that he doesn't like to load firmware in probe(), but in some
situation the drivers have to load firmware in its probe():

        http://marc.info/?l=linux-kernel&m=134592122811866&w=2

In fact, it is better for drivers to load firmware just when user wants to use
the device, and some drivers have already changed to load firmware in
the open() callback.

So looks loading firmware always before probe in driver core is
against the above idea.

> firmware load off and only when the firmware had loaded would it call
> ->probe with dev->firmware pointing at a refcounted firmware struct.

IMO, introduce refcount for the firmware doesn't make sense. The lifetime
of firmware is completely different with lifetime of driver or device:

    - firmware needn't be kept in memory in the device/driver's lifetime, and
      should be loaded just in need, and be released after downloading
      it into device.

   - sometimes devices may disappear, but it is better to keep the
     firmware in memory, for example, device may be disconnected
     during resume but will come back later.

>
> 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

As said above, ref/deref on probe/remove is not a good idea since
we needn't to keep the firmware in memory during the whole device/driver
lifetime.

Thanks,
--
Ming Lei
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists