Ulf Hansson <ulf.hansson@xxxxxxxxxx> writes: > The initial state of the device's need_restore flag should'nt depend on > the current state of the PM domain. For example it should be perfectly > valid to attach an inactive device to a powered PM domain. > > The pm_genpd_dev_need_restore() API allow us to update the need_restore > flag to somewhat cope with such scenarios. Typically that should have > been done from drivers/buses ->probe() since it's those that put the > requirements on the value of the need_restore flag. > > Until recently, the Exynos SOCs were the only user of the > pm_genpd_dev_need_restore() API, though invoking it from a centralized > location while adding devices to their PM domains. > > Due to that Exynos now have swithed to the generic OF-based PM domain > look-up, it's no longer possible to invoke the API from a centralized > location. The reason is because devices are now added to their PM > domains during the probe sequence. > > Commit "ARM: exynos: Move to generic PM domain DT bindings" > did the switch for Exynos to the generic OF-based PM domain look-up, > but it also removed the call to pm_genpd_dev_need_restore(). This > caused a regression for some of the Exynos drivers. > > To handle things more properly in the generic PM domain, let's change > the default initial value of the need_restore flag to reflect that the > state is unknown. As soon as some of the runtime PM callbacks gets > invoked, update the initial value accordingly. > > Moreover, since the generic PM domain is verifying that all device's > are both runtime PM enabled and suspended, using pm_runtime_suspended() > while pm_genpd_poweroff() is invoked from the scheduled work, we can be > sure of that the PM domain won't be powering off while having active > devices. > > Do note that, the generic PM domain can still only know about active > devices which has been activated through invoking its runtime PM resume > callback. In other words, buses/drivers using pm_runtime_set_active() > during ->probe() will still suffer from a race condition, potentially > probing a device without having its PM domain being powered. That issue > will have to be solved using a different approach. > > This a log from the boot regression for Exynos5, which is being fixed in > this patch. > > ------------[ cut here ]------------ > WARNING: CPU: 0 PID: 308 at ../drivers/clk/clk.c:851 clk_disable+0x24/0x30() > Modules linked in: > CPU: 0 PID: 308 Comm: kworker/0:1 Not tainted 3.18.0-rc3-00569-gbd9449f-dirty #10 > Workqueue: pm pm_runtime_work > [<c0013c64>] (unwind_backtrace) from [<c0010dec>] (show_stack+0x10/0x14) > [<c0010dec>] (show_stack) from [<c03ee4cc>] (dump_stack+0x70/0xbc) > [<c03ee4cc>] (dump_stack) from [<c0020d34>] (warn_slowpath_common+0x64/0x88) > [<c0020d34>] (warn_slowpath_common) from [<c0020d74>] (warn_slowpath_null+0x1c/0x24) > [<c0020d74>] (warn_slowpath_null) from [<c03107b0>] (clk_disable+0x24/0x30) > [<c03107b0>] (clk_disable) from [<c02cc834>] (gsc_runtime_suspend+0x128/0x160) > [<c02cc834>] (gsc_runtime_suspend) from [<c0249024>] (pm_generic_runtime_suspend+0x2c/0x38) > [<c0249024>] (pm_generic_runtime_suspend) from [<c024f44c>] (pm_genpd_default_save_state+0x2c/0x8c) > [<c024f44c>] (pm_genpd_default_save_state) from [<c024ff2c>] (pm_genpd_poweroff+0x224/0x3ec) > [<c024ff2c>] (pm_genpd_poweroff) from [<c02501b4>] (pm_genpd_runtime_suspend+0x9c/0xcc) > [<c02501b4>] (pm_genpd_runtime_suspend) from [<c024a4f8>] (__rpm_callback+0x2c/0x60) > [<c024a4f8>] (__rpm_callback) from [<c024a54c>] (rpm_callback+0x20/0x74) > [<c024a54c>] (rpm_callback) from [<c024a930>] (rpm_suspend+0xd4/0x43c) > [<c024a930>] (rpm_suspend) from [<c024bbcc>] (pm_runtime_work+0x80/0x90) > [<c024bbcc>] (pm_runtime_work) from [<c0032a9c>] (process_one_work+0x12c/0x314) > [<c0032a9c>] (process_one_work) from [<c0032cf4>] (worker_thread+0x3c/0x4b0) > [<c0032cf4>] (worker_thread) from [<c003747c>] (kthread+0xcc/0xe8) > [<c003747c>] (kthread) from [<c000e738>] (ret_from_fork+0x14/0x3c) > ---[ end trace 40cd58bcd6988f12 ]--- > > Fixes: a4a8c2c4962bb655 (ARM: exynos: Move to generic PM domain DT bindings) > Reported-by: Sylwester Nawrocki <s.nawrocki@xxxxxxxxxxx> > Signed-off-by: Ulf Hansson <ulf.hansson@xxxxxxxxxx> Reviewed-by: Kevin Hilman <khilman@xxxxxxxxxx> And looks like we need this for v3.18-rc since it does fix the regression. A minor nit: you add a few new multi-line comment blocks which should have a new empty line before them, but currently they're right up against the previous code. Please add a blank line for separation and to be consisitent with the rest of the file. Thanks, 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
 |