Re: loading firmware while usermodehelper disabled.

Larry Finger
2012-01-01T03:49:16Z

On 12/30/2011 06:22 PM, Linus Torvalds wrote:
> On Fri, Dec 30, 2011 at 3:54 PM, Dave Jones<davej@redhat.com> wrote:
>> We're getting a bunch of reports against Fedora 16
>> (still using 3.1) which look like some drivers are trying to
>> load firmware on resume from suspend, while usermodehelper
>> is disabled.
>
> Ok, buggy drivers. You *must*not* load firmware in your resume path,
> since there is no actual guarantee that any particular device will be
> resumed after the disk that contains the firmware images.
>
> So it's very simple: drivers that load firmware at resume time are
> buggy. No ifs, buts, or maybes about it.
>
>> Here are some example traces:
>>
>> https://bugzilla.redhat.com/show_bug.cgi?id=746411
>
> It's isight_firmware_load(), in the isight_firmware driver. The driver
> doesn't actually do anything but load the firmware, and is apparently
> not very good at that either.
>
> It should either fake a disconnect and reconnect of the device (and
> let the reconnect then load the firmware through udev or something) or
> it should just save the firmware image in memory from the original
> load, and make the resume just re-initialize it - not load it.
>
> It's also possible that it could be considered a USB layer bug, and
> the USB layer should just not rebind the devices directly in the
> resume function, but do it somehow later. HOWEVER, that would only
> work for "random" USB devices that aren't in use by user space (like
> disks etc might be). So I think that in general the real solution is
> always just "make sure that the firmware is in memory before the
> suspend even happens".
>
> Greg - has the USB resume logic been changed lately?
>
> Matthew? Any comments about that particular driver?
>
>> https://bugzilla.redhat.com/show_bug.cgi?id=771002
>
> Same issue, different driver. Again, it's USB, and it's possible that
> USB just makes it really hard to do this correctly (ie the "save
> firmware image across suspend so that you don't have to load it at
> resume time").
>
> It's also possible that we should blame the firmware code, which is
> expressly written to encourage these kinds of bugs. It may be that i
> tshould be the firmware code that has a "get_firmware()" +
> "put_firmware()" model, and it should cache the firmware explicitly if
> the config supports suspend, so that a firmware read at resume time
> would actually work. The whole "request_firmware()" interface really
> is very prone to these kinds of bugs.
>
> But it's possible this could be fixed at the driver level by doing the
> caching there.
>
> In this case it's the rtl8192cu driver, so Larry, Chaoming, John etc
> added to the cc for that one.

A patch that delays the firmware reading until the upload is needed when the
interface is brought up will be sent to John very soon. My box loses its video
when a suspend/resume cycle is done, but the WARNING is no longer printed by
rtl8192cu.

Larry