On Mon, 13 Jun 2011, Kenneth Heitke wrote: > > If runtime PM isn't present, why do you want to manage your clocks etc. > > at all? The fact that it's not in the kernel means the system manager > > doesn't care about power usage. > > I am trying to be backwards compatible. There is likely a period of > time from when the runtime PM feature was added to when it was turned on > by default. If the feature happens to be disabled, I think it makes > sense for the driver to still do what it can to manage its resources. > The power guys aren't going to let me off the hook that easily :) Maybe not... but then you can point out that it's somebody else's fault! :-) More to the point, runtime PM should be enabled at the same time it is added. The code for both belongs in the subsystem and driver, so there's no reason not to do them together. IMO it's silly to bypass the runtime PM core's notion of when power management should or should not be available. If you're going to do that, why bother using the runtime PM framework in the first place? > > It's up to the driver and the subsystem, since they are the entities > > that are responsible for disabling runtime PM. If you think disabling > > runtime PM will cause problems, then don't do it. > > I'm thinking about within runtime PM itself. I believe during system > suspend, disable() followed by enable() can be called. If that happens, > are there any scenarios that I need to be concerned about? Can my > autosuspend timer just happen to fire during that window between disable > and enable resulting in a failure to suspend? My driver is part of the > i2c subsystem, do I know for a fact that disable() won't be used? Ah -- that's a very good question. The PM core doesn't call disable followed by enable, but subsystems do as part of system resume. In fact, I'm not at all sure that the current implementation is correct in this regard. The failure scenario you bring up seems entirely possible. I think we need to have the PM core call pm_runtime_get_noresume() before invoking the resume_noirq (or thaw_noirq or restore_noirq) callback, and then call pm_runtime_put_sync() after invoking the complete callback. This would solve your race: The put_sync would invoke the runtime_idle method, which would start another runtime suspend or autosuspend. (It used to be that the PM core called pm_runtime_get_noresume() earlier on, before the prepare callback. This also solved your race, but it caused other problems and so was changed.) It's true that subsystems could do this for themselves, but then they'd _all_ have to do it. So we might as well put it in the PM core. Rafael, what do you think? Alan Stern _______________________________________________ linux-pm mailing list linux-pm@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linux-foundation.org/mailman/listinfo/linux-pm
