diff mbox series

PM / wakeup: Unify device_init_wakeup() for PM_SLEEP and !PM_SLEEP

Message ID 20220607035158.308111-1-helgaas@kernel.org
State New
Headers show
Series PM / wakeup: Unify device_init_wakeup() for PM_SLEEP and !PM_SLEEP | expand

Commit Message

Bjorn Helgaas June 7, 2022, 3:51 a.m. UTC
From: Bjorn Helgaas <bhelgaas@google.com>

Previously the CONFIG_PM_SLEEP and !CONFIG_PM_SLEEP device_init_wakeup()
implementations differed in confusing ways:

  - The PM_SLEEP version checked for a NULL device pointer and returned
    -EINVAL, while the !PM_SLEEP version did not and would simply
    dereference a NULL pointer.

  - When called with "false", the !PM_SLEEP version cleared "capable" and
    "enable" in the opposite order of the PM_SLEEP version.  That was
    harmless because for !PM_SLEEP they're simple assignments, but it's
    unnecessary confusion.

Use a simplified version of the PM_SLEEP implementation for both cases.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/base/power/wakeup.c | 30 ------------------------------
 include/linux/pm_wakeup.h   | 31 +++++++++++++++++++++++--------
 2 files changed, 23 insertions(+), 38 deletions(-)

Comments

Rafael J. Wysocki July 5, 2022, 6:50 p.m. UTC | #1
On Tue, Jun 7, 2022 at 5:52 AM Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> From: Bjorn Helgaas <bhelgaas@google.com>
>
> Previously the CONFIG_PM_SLEEP and !CONFIG_PM_SLEEP device_init_wakeup()
> implementations differed in confusing ways:
>
>   - The PM_SLEEP version checked for a NULL device pointer and returned
>     -EINVAL, while the !PM_SLEEP version did not and would simply
>     dereference a NULL pointer.
>
>   - When called with "false", the !PM_SLEEP version cleared "capable" and
>     "enable" in the opposite order of the PM_SLEEP version.  That was
>     harmless because for !PM_SLEEP they're simple assignments, but it's
>     unnecessary confusion.
>
> Use a simplified version of the PM_SLEEP implementation for both cases.
>
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> ---
>  drivers/base/power/wakeup.c | 30 ------------------------------
>  include/linux/pm_wakeup.h   | 31 +++++++++++++++++++++++--------
>  2 files changed, 23 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/base/power/wakeup.c b/drivers/base/power/wakeup.c
> index 11a4ffe91367..e3befa2c1b66 100644
> --- a/drivers/base/power/wakeup.c
> +++ b/drivers/base/power/wakeup.c
> @@ -500,36 +500,6 @@ void device_set_wakeup_capable(struct device *dev, bool capable)
>  }
>  EXPORT_SYMBOL_GPL(device_set_wakeup_capable);
>
> -/**
> - * device_init_wakeup - Device wakeup initialization.
> - * @dev: Device to handle.
> - * @enable: Whether or not to enable @dev as a wakeup device.
> - *
> - * By default, most devices should leave wakeup disabled.  The exceptions are
> - * devices that everyone expects to be wakeup sources: keyboards, power buttons,
> - * possibly network interfaces, etc.  Also, devices that don't generate their
> - * own wakeup requests but merely forward requests from one bus to another
> - * (like PCI bridges) should have wakeup enabled by default.
> - */
> -int device_init_wakeup(struct device *dev, bool enable)
> -{
> -       int ret = 0;
> -
> -       if (!dev)
> -               return -EINVAL;
> -
> -       if (enable) {
> -               device_set_wakeup_capable(dev, true);
> -               ret = device_wakeup_enable(dev);
> -       } else {
> -               device_wakeup_disable(dev);
> -               device_set_wakeup_capable(dev, false);
> -       }
> -
> -       return ret;
> -}
> -EXPORT_SYMBOL_GPL(device_init_wakeup);
> -
>  /**
>   * device_set_wakeup_enable - Enable or disable a device to wake up the system.
>   * @dev: Device to handle.
> diff --git a/include/linux/pm_wakeup.h b/include/linux/pm_wakeup.h
> index 196a157456aa..77f4849e3418 100644
> --- a/include/linux/pm_wakeup.h
> +++ b/include/linux/pm_wakeup.h
> @@ -109,7 +109,6 @@ extern struct wakeup_source *wakeup_sources_walk_next(struct wakeup_source *ws);
>  extern int device_wakeup_enable(struct device *dev);
>  extern int device_wakeup_disable(struct device *dev);
>  extern void device_set_wakeup_capable(struct device *dev, bool capable);
> -extern int device_init_wakeup(struct device *dev, bool val);
>  extern int device_set_wakeup_enable(struct device *dev, bool enable);
>  extern void __pm_stay_awake(struct wakeup_source *ws);
>  extern void pm_stay_awake(struct device *dev);
> @@ -167,13 +166,6 @@ static inline int device_set_wakeup_enable(struct device *dev, bool enable)
>         return 0;
>  }
>
> -static inline int device_init_wakeup(struct device *dev, bool val)
> -{
> -       device_set_wakeup_capable(dev, val);
> -       device_set_wakeup_enable(dev, val);
> -       return 0;
> -}
> -
>  static inline bool device_may_wakeup(struct device *dev)
>  {
>         return dev->power.can_wakeup && dev->power.should_wakeup;
> @@ -217,4 +209,27 @@ static inline void pm_wakeup_hard_event(struct device *dev)
>         return pm_wakeup_dev_event(dev, 0, true);
>  }
>
> +/**
> + * device_init_wakeup - Device wakeup initialization.
> + * @dev: Device to handle.
> + * @enable: Whether or not to enable @dev as a wakeup device.
> + *
> + * By default, most devices should leave wakeup disabled.  The exceptions are
> + * devices that everyone expects to be wakeup sources: keyboards, power buttons,
> + * possibly network interfaces, etc.  Also, devices that don't generate their
> + * own wakeup requests but merely forward requests from one bus to another
> + * (like PCI bridges) should have wakeup enabled by default.
> + */
> +static inline int device_init_wakeup(struct device *dev, bool enable)
> +{
> +       if (enable) {
> +               device_set_wakeup_capable(dev, true);
> +               return device_wakeup_enable(dev);
> +       } else {
> +               device_wakeup_disable(dev);
> +               device_set_wakeup_capable(dev, false);
> +               return 0;
> +       }
> +}
> +
>  #endif /* _LINUX_PM_WAKEUP_H */
> --

Applied as 5.20 material, sorry for the delay.

Thanks!
diff mbox series

Patch

diff --git a/drivers/base/power/wakeup.c b/drivers/base/power/wakeup.c
index 11a4ffe91367..e3befa2c1b66 100644
--- a/drivers/base/power/wakeup.c
+++ b/drivers/base/power/wakeup.c
@@ -500,36 +500,6 @@  void device_set_wakeup_capable(struct device *dev, bool capable)
 }
 EXPORT_SYMBOL_GPL(device_set_wakeup_capable);
 
-/**
- * device_init_wakeup - Device wakeup initialization.
- * @dev: Device to handle.
- * @enable: Whether or not to enable @dev as a wakeup device.
- *
- * By default, most devices should leave wakeup disabled.  The exceptions are
- * devices that everyone expects to be wakeup sources: keyboards, power buttons,
- * possibly network interfaces, etc.  Also, devices that don't generate their
- * own wakeup requests but merely forward requests from one bus to another
- * (like PCI bridges) should have wakeup enabled by default.
- */
-int device_init_wakeup(struct device *dev, bool enable)
-{
-	int ret = 0;
-
-	if (!dev)
-		return -EINVAL;
-
-	if (enable) {
-		device_set_wakeup_capable(dev, true);
-		ret = device_wakeup_enable(dev);
-	} else {
-		device_wakeup_disable(dev);
-		device_set_wakeup_capable(dev, false);
-	}
-
-	return ret;
-}
-EXPORT_SYMBOL_GPL(device_init_wakeup);
-
 /**
  * device_set_wakeup_enable - Enable or disable a device to wake up the system.
  * @dev: Device to handle.
diff --git a/include/linux/pm_wakeup.h b/include/linux/pm_wakeup.h
index 196a157456aa..77f4849e3418 100644
--- a/include/linux/pm_wakeup.h
+++ b/include/linux/pm_wakeup.h
@@ -109,7 +109,6 @@  extern struct wakeup_source *wakeup_sources_walk_next(struct wakeup_source *ws);
 extern int device_wakeup_enable(struct device *dev);
 extern int device_wakeup_disable(struct device *dev);
 extern void device_set_wakeup_capable(struct device *dev, bool capable);
-extern int device_init_wakeup(struct device *dev, bool val);
 extern int device_set_wakeup_enable(struct device *dev, bool enable);
 extern void __pm_stay_awake(struct wakeup_source *ws);
 extern void pm_stay_awake(struct device *dev);
@@ -167,13 +166,6 @@  static inline int device_set_wakeup_enable(struct device *dev, bool enable)
 	return 0;
 }
 
-static inline int device_init_wakeup(struct device *dev, bool val)
-{
-	device_set_wakeup_capable(dev, val);
-	device_set_wakeup_enable(dev, val);
-	return 0;
-}
-
 static inline bool device_may_wakeup(struct device *dev)
 {
 	return dev->power.can_wakeup && dev->power.should_wakeup;
@@ -217,4 +209,27 @@  static inline void pm_wakeup_hard_event(struct device *dev)
 	return pm_wakeup_dev_event(dev, 0, true);
 }
 
+/**
+ * device_init_wakeup - Device wakeup initialization.
+ * @dev: Device to handle.
+ * @enable: Whether or not to enable @dev as a wakeup device.
+ *
+ * By default, most devices should leave wakeup disabled.  The exceptions are
+ * devices that everyone expects to be wakeup sources: keyboards, power buttons,
+ * possibly network interfaces, etc.  Also, devices that don't generate their
+ * own wakeup requests but merely forward requests from one bus to another
+ * (like PCI bridges) should have wakeup enabled by default.
+ */
+static inline int device_init_wakeup(struct device *dev, bool enable)
+{
+	if (enable) {
+		device_set_wakeup_capable(dev, true);
+		return device_wakeup_enable(dev);
+	} else {
+		device_wakeup_disable(dev);
+		device_set_wakeup_capable(dev, false);
+		return 0;
+	}
+}
+
 #endif /* _LINUX_PM_WAKEUP_H */