On Tue, Jan 2, 2018 at 5:08 PM, Ulf Hansson <ulf.hansson@xxxxxxxxxx> wrote:
> Currently the wakeup_path status flag becomes propagated from a child
> device to its parent device at __device_suspend(). This allows a driver
> dealing with a parent device to act on the flag from its ->suspend()
> callback.
>
> However, in situations when the wakeup_path status flag needs to be set
> from a ->suspend_late() callback, its value doesn't get propagated to the
> parent by the PM core. Let's address this limitation, by also propagating
> the flag at __device_suspend_late().
This doesn't need to be done twice, so doing it once in
__device_suspend_late() should be sufficient.
> Signed-off-by: Ulf Hansson <ulf.hansson@xxxxxxxxxx>
> ---
> drivers/base/power/main.c | 33 +++++++++++++++++----------------
> 1 file changed, 17 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
> index 7aeb91d..612bf1b 100644
> --- a/drivers/base/power/main.c
> +++ b/drivers/base/power/main.c
> @@ -1476,6 +1476,22 @@ static pm_callback_t dpm_subsys_suspend_late_cb(struct device *dev,
> return callback;
> }
>
> +static void dpm_propagate_to_parent(struct device *dev)
> +{
> + struct device *parent = dev->parent;
> +
> + if (!parent)
> + return;
> +
> + spin_lock_irq(&parent->power.lock);
> +
> + parent->power.direct_complete = false;
> + if (dev->power.wakeup_path && !parent->power.ignore_children)
> + parent->power.wakeup_path = true;
> +
> + spin_unlock_irq(&parent->power.lock);
> +}
Clearing the parent's direct_complete in __device_suspend_late() is
incorrect, because it potentially overwrites the value previously used
by __device_suspend() for the parent.
IMO it also need not be done under parent->power.lock, however,
because the other parent's power. flags should not be updated in
parallel with this update anyway, so maybe just move the parent's
direct_complete update to __device_suspend(), rename
dpm_propagate_to_parent() to something like
dpm_propagate_wakeup_to_parent() and call it from
__device_suspend_late() only?
Thanks,
Rafael
