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. 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. > > The early bood situation may be handled through deferred probing. Yes, deferring it will make drivers more friendly and be easy to use. [1], http://marc.info/?l=linux-usb&m=132554118928398&w=2 thanks, -- Ming Lei -- 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
