[...] >> >> Scenario 5), a platform driver with/without runtime PM callbacks. >> ->probe() >> - do some initialization >> - may fetch handles to runtime PM resources >> - pm_runtime_enable() > > Well, and now how the driver knows if the device is "on" before accessing it? In this case the driver don't need to access the device during ->probe(). That's postponed until sometime later. > >> Note 1) >> Scenario 1) and 2), both relies on the approach to power on the PM >> domain by using pm_runtime_get_sync(). That approach didn't work when >> CONFIG_PM_RUNTIME was unset, but we recently decided to fixed that by >> the below patch, so that's good! >> "[PATCH] PM / domains: Kconfig: always enable PM_RUNTIME when genpd enabled" >> >> Note 2) >> Scenario 3) and 4) use the same principles for managing runtime PM. >> These scenarios needs a way to power on the generic PM domain prior >> probing the device. The call to pm_runtime_set_active(), prevents an >> already powered PM domain from power off until after probe, but that's >> not enough. >> >> Note 3) >> The $subject patch, tried to address the issues for scenario 3) and >> 4). It does so, but will affect scenario 5) which was working nicely >> before. In scenario 5), the $subject patch will cause the generic PM >> domain to potentially stay powered after ->probe() even if the device >> is runtime PM suspended. > > Why would it? If the device is runtime-suspended, the domain will know > that, because its callbacks will be used for that. At least, that's > what I'd expect to happen, so is there a problem here? Genpd do knows about the device but it doesn’t get a "notification" to power off. There are no issues whatsoever for driver. This is a somewhat special case. Let's go through an example. 1. The PM domain is initially in powered off state. 2. The bus ->probe() invokes dev_pm_domain_attach() and then the PM domain gets attached to the device. 3. $subject patch causes the PM domain to power on. 4. A driver ->probe() sequence start, following the Scenario 5). 5. The device is initially in runtime PM suspended state and it will remain so during ->probe(). 6. The pm_request_idle() invoked after really_probe() in driver core, won't trigger a runtime PM suspend callback to be invoked. In other words, genpd don't get a "notification" that it may power off. In this state, genpd will either power off from the late_initcall, genpd_poweroff_unused() (depending on when the driver was probed) or wait until some device's runtime PM suspend callback is invoked at any later point. > >> I see three options going forward. >> >> Option 1) >> Convert scenario 3) and 4) into using the pm_runtime_get_sync() >> approach. There are no theoretical obstacles to do this, but pure >> practical. There are a lot of drivers that needs to be converted and >> we also need to teach driver authors future wise to not use >> pm_runtime_set_active() in this manner. > > I'd say we need to do something like this anyway. That is, standardize on > *one* approach. I'm actually not sure what approach is the most useful, > but the pm_runtime_get_sync() one seems to be the most popular to me. > >> Option 2) >> Add some kind of get/put API for PM domains. The buses invokes it to >> control the power to the PM domain. From what I understand, that's >> also what Dmitry think is needed!? >> Anyway, that somehow means to proceed with the approach I took in the >> below patchset. >> [PATCH v3 0/9] PM / Domains: Fix race conditions during boot >> http://marc.info/?t=141320907000003&r=1&w=2 > > I don't like that. The API is already quite complicated in my view and > adding even more complexity to it is not going to help in the long run. I absolutely agree that we shouldn't add unnecessary APIs and keep APIs as simple as possible. In that context, I think the effect from proceeding with Option 2) also means there are no need for the below APIs any more. pm_genpd_poweron() pm_genpd_name_poweron() (requires some additional work though) pm_genpd_poweroff_unused() pm_genpd_dev_need_restore() I guess you figured out that I am in favour of Option 2). :-) Especially since it cover all scenarios and we don't have to go a fix a vast amount of drivers. > >> Option 3) >> Live we the limitation this $subject patch introduces for scenario 5). > > I'd say, 3) for now and 1) going forward. This could work! The hardest part is to know when we should revert $subject patch, to fix the regression introduced for scenario 5). > >> Is there maybe any additional options, that I didn't think of? >> >> Apologize for the long email, I hope I didn't bored you too much. > > That's fine, thanks for taking the time to lay out all of the cases, that's > always helpful. > > Rafael > 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
 |