Ulf Hansson <ulf.hansson@xxxxxxxxxx> writes: > On 3 October 2014 03:14, Kevin Hilman <khilman@xxxxxxxxxx> wrote: >> Ulf, Rafael, >> >> Ulf Hansson <ulf.hansson@xxxxxxxxxx> writes: >> >>> When there are more than one device in a PM domain these will obviously >>> be probed at different times. Depending on timing and the implemented >>> support for runtime PM in a driver/subsystem, genpd may be advised to >>> power off a PM domain after a successful probe sequence. >>> >>> Ideally we should have relied on the driver/subsystem, through runtime >>> PM, to bring their device's PM domain into powered state prior doing >>> probing if such requirement exist. >> >> I think I've stumbled on a related problem, or maybe the same one. >> >> Even if platform-specific init code has initialized a device with >> pm_runtime_set_active(), it seems that the genpd domain can still >> power off before before all of its devices are probed. >> >> This is because pm_genpd_poweroff() requires there to be a driver >> when it's checking if a device is pm_runtime_suspended() which will not >> be the case if the driver has not been probed yet. >> >> Consider this case: There are several devices in the domain that haven't >> been probed yet (dev->driver == NULL), but have been marked with >> pm_runtime_set_active() + _get_noresume(), so pm_runtime_suspended() == false. > > I haven't seen this kind of set up before. Are you invoking > pm_runtime_enable() here as well? Yes: _set_active(), _get_noresume() and _enable(). > I am not sure pm_runtime_get_noresume() is a good idea, since that > will prevent the device from going inactive - even after the driver > has probed it. That's the goal. The experiment I'm doing is the equivalent of a _get_sync() in ->probe and a _put() in ->remove. > Unless the driver do pm_runtime_put_sync twice of > course. :-) > > On the other hand, if you have done pm_runtime_enable() your certainly > need to prevent the device some going inactive... Exactly. >> >> Then, one of devices is in the domain is probed, and during the probe it >> does a _get_sync(), sets some stuff up, and then does _put_sync(). >> After the probe, because of the _put_sync(), the genpd >> ->runtime_suspend() will be triggered, causing it to attempt a >> _genpd_poweroff(). Since the rest of the devices in the domain haven't >> (yet) been probed, their dev->driver pointers are all still NULL, so the >> pm_runtime_suspended() check will not be attempted for them. >> >> The result is that the genpd will poweroff after the first device is >> probed, but before the others have had a chance to probe, which is not >> exactly desired behavior for a genpd that has been initialized as >> powered on. >> >> With the hack below[1], I'm able to avoid that problem, but am not >> completely sure yet if this is safe in general. >> >> Rafael, do you remember why that check for dev->driver is needed? >> Without digging deeper (which I'll do tomorrow), seems to me that >> checking pm_runtime_suspended() on devices without drivers is a >> reasonable thing to do since they can be initailzed by platform code >> before they are probed. If you think this is OK, I'll cook up a real >> patch with a changelog. >> >> Ulf, I'm not sure if this is the same problem you're having, but do you >> think this would solve your problem if the drivers are properly >> initialized? > > Unfortunately no. > > I am using the DT initialization path so all my devices aren't being > added to the PM domain before drivers starts to probe them. > > Instead they are added when each device gets probed, thus the PM > domain can still power off between devices being probed. OK Kevin -- 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
 |