On Saturday, July 21, 2012, Ming Lei wrote: > On Sat, Jul 21, 2012 at 5:56 PM, Rafael J. Wysocki <rjw@xxxxxxx> wrote: > > On Saturday, July 21, 2012, Ming Lei wrote: > >> CC guys who discussed the problem in the below link in Jan. : > >> > >> http://marc.info/?t=132528956000002&r=10&w=2 > >> > >> On Fri, Jul 20, 2012 at 8:33 PM, Ming Lei <tom.leiming@xxxxxxxxx> wrote: > >> > The RFC patch is just for discussing if the idea of deferring > >> > request_firmware is doable for addressing the issue of > >> > request_firmware in resume path, which is caused by driver > >> > unbind/rebind during resume. > >> > > >> > At least usb bus is involved in such things, one driver may be > >> > unbound and rebound in resume path at several situations, and > >> > request_firmware is often called inside probe(). > >> > > >> > Also the idea should be helpful for other hotplug buses too, > >> > at least there was the similar problem report on pcmcia bus. > >> > >> Looks it works well in my two test cases: one is to call request_firmware > >> in early boot(initcall), another one is to call request_firmware in probe() > >> of resume path(caused by unbind & rebind). And without the patch, both > >> two request_firmware return failure and can't complete the loading. > > > > And that's precisely why you're not supposed to use request_firmware() in > > those two situations. > > So you mean we should do as below? > > For the early boot situation, the driver which calls request_firmware in probe() > can't be built in kernel image. > > For the second situation, some drivers can't be allowed to call > request_firmware() > in its probe() because these drivers may be unbound and rebound inside resume() > or complete_resume(), for example, see usb_resume_complete(). > > IMO, the 1st one is very unfriendly and the 2nd one can't be avoided for some > hotplug buses. I'm not sure if it really can't be avoided. > That is just the motivation of this patch to make request_firmware() workable in > both the two above situations. > > > > > My opinion is that, at least for the suspend/hibernate case, the firmware > > data should be loaded into memory before suspend (e.g. using a PM notifier) > > and released only after the corresponding resume (or suspend failure), so > > that it's present in memory during the entire suspend-resume cycle. > > The patch isn't to replace caching firmware data during suspend-resume cycle, > and just a supplement for it. > > It is not easy to cache firmware data during suspend-resume cycle for > the above 2nd situation because of the lifetime problem of firmware data: > the driver may be unbound and rebound inside resume path, even the > device may vanish and appear again. > > Also, Matthew had a below case[1] which can't be solved with caching > firmware data at all, not to mention consuming much memory by caching > firmware: > > 1) user boots from cold. Device comes up with generic USB ID. > 2) isight_firmware loads and binds. Firmware is loaded. Device > disconnects and reconnects with an ID that's bound by the UVC > driver. > 3) user reboots. Device comes up with UVC ID. isight_firmware > doesn't bind. > 4) user suspends. > 5) user resumes. isight_firmware binds and attempts to load firmware. > > But it can be dealt with easily by the simple patch. > > Finally, suppose caching firmware may work well for the 2nd situation, we still > have to cache all the firmwares of all hotplug devices(in one system) which > need firmware before suspending, because these devices may be unplugged > and plugged again during suspend-resume cycle or be powered off by system. OK, I give up. This may not be too ugly to live, after all. I'll post some comments in a reply to the message with the patch. Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
