On Fri, Dec 29, 2017 at 12:37 PM, Ulf Hansson <ulf.hansson@xxxxxxxxxx> wrote:
> In general, wakeup settings are not supposed to be changed during any of
> the system wide PM phases. The reason is simply that it would break
> guarantees provided by the PM core, to properly act on active wakeup
> sources.
>
> However, there are exceptions to when, in particular, disabling a device as
> wakeup source makes sense. For example, in cases when a driver realizes
> that its device is dead during system suspend. For these scenarios, we
> don't need to care about acting on the wakeup source correctly, because a
> dead device shouldn't deliver wakeup signals.
>
> To this reasoning and to help users to properly manage wakeup settings,
> let's print a warning in cases someone calls device_wakeup_enable() during
> system sleep.
>
> Signed-off-by: Ulf Hansson <ulf.hansson@xxxxxxxxxx>
> ---
> drivers/base/power/main.c | 4 ++++
> drivers/base/power/wakeup.c | 4 ++++
> include/linux/pm.h | 1 +
> 3 files changed, 9 insertions(+)
>
> diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
> index 1327726..6b0d312 100644
> --- a/drivers/base/power/main.c
> +++ b/drivers/base/power/main.c
> @@ -1030,6 +1030,8 @@ static void device_complete(struct device *dev, pm_message_t state)
> callback(dev);
> }
>
> + dev->power.may_set_wakeup = dev->power.can_wakeup;
> +
> device_unlock(dev);
>
> pm_runtime_put(dev);
> @@ -1747,6 +1749,7 @@ static int device_prepare(struct device *dev, pm_message_t state)
>
> device_lock(dev);
>
> + dev->power.may_set_wakeup = false;
> dev->power.wakeup_path = false;
>
> if (dev->power.no_pm_callbacks) {
> @@ -1775,6 +1778,7 @@ static int device_prepare(struct device *dev, pm_message_t state)
> if (ret < 0) {
> suspend_report_result(callback, ret);
> pm_runtime_put(dev);
> + dev->power.may_set_wakeup = dev->power.can_wakeup;
> return ret;
> }
> /*
> diff --git a/drivers/base/power/wakeup.c b/drivers/base/power/wakeup.c
> index cb72965..5445bea 100644
> --- a/drivers/base/power/wakeup.c
> +++ b/drivers/base/power/wakeup.c
> @@ -268,6 +268,9 @@ int device_wakeup_enable(struct device *dev)
> if (!dev || !dev->power.can_wakeup)
> return -EINVAL;
>
> + if (!dev->power.may_set_wakeup)
> + dev_warn(dev, "don't enable as wakup source during sleep!\n");
> +
> ws = wakeup_source_register(dev_name(dev));
> if (!ws)
> return -ENOMEM;
> @@ -413,6 +416,7 @@ void device_set_wakeup_capable(struct device *dev, bool capable)
> return;
>
> dev->power.can_wakeup = capable;
> + dev->power.may_set_wakeup = capable;
> if (device_is_registered(dev) && !list_empty(&dev->power.entry)) {
> if (capable) {
> int ret = wakeup_sysfs_add(dev);
> diff --git a/include/linux/pm.h b/include/linux/pm.h
> index ebc6ef8..106fb15 100644
> --- a/include/linux/pm.h
> +++ b/include/linux/pm.h
> @@ -606,6 +606,7 @@ struct dev_pm_info {
> struct list_head entry;
> struct completion completion;
> struct wakeup_source *wakeup;
> + bool may_set_wakeup:1;
> bool wakeup_path:1;
> bool syscore:1;
> bool no_pm_callbacks:1; /* Owned by the PM core */
> --
I didn't mean anything so complicated. :-)
Just a warning message if (pm_suspend_target_state != PM_SUSPEND_ON)
in device_wakeup_enable() (or equivalent) should be sufficient.
