[...] > Generally, there are two or even three levels of runtime PM handling, > driver, (possibly) bus type and (possibly) PM domain (and multiple levels > of these are possible in principle). All of them have to be initialized > at different times. > > Quite arguably, the PM domain and/or bus type runtime PM handling should > be initialized even before registerind the device or during device > registration. Doing that later may be too late. When the device has been > registered, runtime PM should work to an extent allowing the driver to access > the device and configure it further after calling pm_runtime_resume(). > > Of course, if ->probe() is to call pm_runtime_resume() for this purpose, > it must take the fact that the driver's own ->runtime_resume() may be called > as a result of this into account. That's why I'm asking whether or not the > core should call pm_runtime_resume() before calling really_probe() in a > followup branch of this thread. I am reading the other thread, let's see. > > The driver's own runtime PM handling must be initialized in the driver and > the only place suitable for that is ->probe(). However, it needs to be done > *before* the driver's own ->runtime_resume() or ->runtime_suspend() callback > is executed. If that is done properly, it should be possible to cover > both the CONFIG_PM_RUNTIME set/unset cases in that code. > > And I wouldn't recommend anyone to do the runtime PM initialization in > ->runtime_resume() (when it is called for the first time), as that would be > error prone and fragile. Great! That's means we are at least aligned on this topic. :-) > >> The AMBA bus and some of its drivers a good example of how this has >> been implemented: >> driver/amba/bus.c >> drivers/mmc/host/mmci.c >> drivers/spi/spi-pl022.c >> >> This conclusion I have made from this is: >> - Using pm_runtime_get_sync() during the ->probe() path to explicitly >> power up a PM domain, is not suitable as the _common_ solution to >> solve the race condition. It certainly may work for some scenarios, >> but not for those I am looking at. > > I think, however, that it might work if the core calls pm_runtime_get_sync() > from driver_probe_device(). Currently this won't work. That's because the buses' ->probe() are invoked in this path and they are doing the attachment of the device to its PM domain. In other words, we can't power up the PM domain using pm_runtime_get_sync(), until the device has been attached to its PM domain. Right? [...] >> For PM domains that are initialized in powered off state, we can't >> rely on CONFIG_PM_RUNTIME and thus not on pm_runtime_get_sync() to >> power on these PM domains. We need a different mechanism, which is >> suggested in this v3 patchset. > > That is quite simple to address, though. You can register a bus type > notifier that will power up the domain on BUS_NOTIFY_ADD_DEVICE events > (where the target device belongs to the domain), and do that only for > CONFIG_PM_RUNTIME unset (otherwise runtime PM should take care of this). I guess we could use notifiers, but I am not sure I see any benefit. The code will be more complex and we need error handling as well. > >> The requirement of being able to initialize PM domains in powered off >> state, was raised during review of v2 of this patchset. I do realize >> that's not easy for you to keep track and remember of all discussions. >> I apologize for not providing this as the topmost important argument >> to why pm_runtime_get_sync() can't be used, in my reply to Kevin. > > OK > > It looks like we need an agreement on what to do and what the expectations > should be for each driver core operation at the high level. That is, > what ->probe() should expect, what ->remove() should expect, what the > state after executing each of them should be for CONFIG_PM_RUNTIME set and > unset etc. Otherwise we'll always have various bus types doing what *they* > think is appropriate and not agreeing with each other. > > -- > I speak only for myself. > Rafael J. Wysocki, Intel Open Source Technology Center. Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
 |