On Tuesday, January 2, 2018 12:36:55 PM CET Ulf Hansson wrote: > On 30 December 2017 at 01:44, Rafael J. Wysocki <[email protected]> wrote: > > On Fri, Dec 29, 2017 at 12:37 PM, Ulf Hansson <[email protected]> > > wrote: > >> The PM core in the device_prepare() phase, resets the wakeup_path status > >> flag to the value of device_may_wakeup(). This means if a ->prepare() or a > >> ->suspend() callback for the device would update the device's wakeup > >> setting, this doesn't become reflected in the wakeup_path status flag. > >> > >> In general this isn't a problem, because wakeup settings are not supposed > >> to be changed (via for example calling device_set_wakeup_enable()) during > >> any system wide suspend/resume phase. Nevertheless there are some users, > >> which can be considered as legacy, that don't conform to this behaviour. > >> > >> These legacy cases should be corrected, however until that is done, let's > >> address the issue from the PM core, by moving the assignment of the > >> wakeup_path status flag to the __device_suspend() phase and after the > >> ->suspend() callback has been invoked. > >> > >> Signed-off-by: Ulf Hansson <[email protected]> > >> --- > >> drivers/base/power/main.c | 4 +++- > >> 1 file changed, 3 insertions(+), 1 deletion(-) > >> > >> diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c > >> index 6e8cc5d..810e5fb 100644 > >> --- a/drivers/base/power/main.c > >> +++ b/drivers/base/power/main.c > >> @@ -1620,6 +1620,8 @@ static int __device_suspend(struct device *dev, > >> pm_message_t state, bool async) > >> End: > >> if (!error) { > >> dev->power.is_suspended = true; > >> + if (device_may_wakeup(dev)) > >> + dev->power.wakeup_path = true; > >> dpm_propagate_to_parent(dev); > >> dpm_clear_suppliers_direct_complete(dev); > >> } > >> @@ -1744,7 +1746,7 @@ static int device_prepare(struct device *dev, > >> pm_message_t state) > >> > >> device_lock(dev); > >> > >> - dev->power.wakeup_path = device_may_wakeup(dev); > >> + dev->power.wakeup_path = false; > > > > If you did "dev->power.wakeup_path = device_may_wakeup(dev)" in > > __device_suspend(), it wouldn't need to be cleared here I guess? > > > > No that doesn't work, because it may override the value of the flag > wrongly, in case a child's wakeup_path flag that is set and has been > propagated to the parent.
I see, OK. So it looks like the new driver flag is not really necessary. The wakeup_path propagation may be done even later, like at the end of __device_suspend_late(), which is fine, because it is not going to be used before __device_suspend_noirq() anyway. Then, drivers can set it from their ->suspend and ->suspend_late callbacks. Thanks, Rafael
